[LyX/master] Make cell index of tabular local for column loop.

[Scott Kostyshak]

On Wed, Feb 12, 2020 at 04:30:23PM +0100, Stephan Witt wrote:
> Am 12.02.2020 um 14:50 schrieb Scott Kostyshak <skostysh at lyx.org>:
> > 
> > On Wed, Feb 12, 2020 at 12:14:07PM +0100, Stephan Witt wrote:
> >> commit e900b61d46f02b2af9afcc697168e47e846b982d
> >> Author: Stephan Witt <switt at lyx.org>
> >> Date:   Wed Feb 12 12:32:31 2020 +0100
> >> 
> >>    Make cell index of tabular local for column loop.
> >> ---
> >> src/insets/InsetTabular.cpp |    4 +---
> >> 1 files changed, 1 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
> >> index bc57e1a..7b4db19 100644
> >> --- a/src/insets/InsetTabular.cpp
> >> +++ b/src/insets/InsetTabular.cpp
> >> @@ -3024,8 +3024,6 @@ void Tabular::TeXRow(otexstream & os, row_type row,
> >> 		     OutputParams const & runparams,
> >> 		     list<col_type> columns, list<col_type> logical_columns) const
> >> {
> >> -	idx_type cell = cellIndex(row, 0);
> >> -
> >> 	//output the top line
> >> 	TeXTopHLine(os, row, columns, logical_columns);
> >> 
> >> @@ -3061,7 +3059,7 @@ void Tabular::TeXRow(otexstream & os, row_type row,
> >> 		if (isPartOfMultiColumn(row, c))
> >> 			continue;
> >> 
> >> -		cell = cellIndex(row, c);
> >> +		idx_type cell = cellIndex(row, c);
> >> 
> >> 		if (isPartOfMultiRow(row, c)
> >> 		    && column_info[c].alignment != LYX_ALIGN_DECIMAL) {
> > 
> > I just ask out curiosity (I still consider myself a beginner in C++):
> > Why make this change? I find the code with your change more readable: in
> > addition to clarifying that cell is used only inside of the for loop, it
> > also makes obvious that the value of cell from the previous iteration is
> > not used in the current iteration.
> 
> IMO, readability is the biggest gain of this style. 
> I’d like the scope of a variable to be as narrow as possible.

That makes sense.

> Another point the compiler mentioned: in case of the loop not
> being entered the assignment before the loop is useless.

Good point, I didn't think about that.

> > Is it true that without optimization, this change would be less
> > efficient because cell has to be recreated (so memory allocated) on each
> > iteration of the for loop? But is the idea that compilers recognize this
> > and only allocate memory once so that in practice there is no loss in
> > efficiency?
> 
> I cannot imagine that optimization of allocated memory is an issue.
> I think the memory is allocated locally on stack anyway.

Thanks for the explanation,

Scott
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20200213/4ce5dd6b/attachment.asc>