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

Stephan Witt st.witt at gmx.net
Wed Feb 12 15:30:23 UTC 2020


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.

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

> 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.

Stephan


More information about the lyx-devel mailing list