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.lu...@suse.cz
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
