Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


On Thursday 02 of February 2012, Stephan Bergmann wrote:
On 02/01/2012 11:40 PM, julien2412 wrote:
Here are the lines :
     961             // dann mal die Tabellenkoepfe raus:
     962             for( SwSelBoxes::iterator it = rBoxes.begin(); it !=
rBoxes.end(); ++it )
     963             {
     964                 pLine = it->second->GetUpper();
     965                 while( pLine->GetUpper() )
     966                     pLine = pLine->GetUpper()->GetUpper();
     967
     968                 if( pTbl->IsHeadline( *pLine ) )
     969                 {
     970                     rBoxes.erase( it++ );
     971                     --it;
     972                 }
     973             }

patch proposed
http://nabble.documentfoundation.org/file/n3708331/sw_patch.txt
sw_patch.txt

The patch solves that, but I can offer a number of remarks:

* I would not replace the for loop with a while one, it unnecessarily
makes the code longer, with the general additional minor drawback of
enlarging the scope of the index variable.  I also would not extract
aEnd; it makes the code longer and requires a tiny additional mental
step to ascertain oneself that it is indeed not invalidated by the loop
body.

* You accidentally changed the indentation of the loop body.

* Hungarian notation is ugly, esp. for trivialities like loop index
variables.

 I agree with all the points, but in Julien's defense, I remember exactly this 
same approach was pushed in recently as a fix to the same issue elsewhere. 
Perhaps we should agree on what the recommended way is? I personally think 
the simplest and most elegant solution is to go with 'it = container.erase( 
it );" and move the "++it" out of for()'s parentheses to an else block of the 
if() condition (oh well, ease of use clearly wasn't one of priorities for STL 
designers).

-- 
 Lubos Lunak
 l.lunak@suse.cz

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.