Re: [Libreoffice] [PATCH][PUSHED] List structure cleaning in calc

Rafael Dominguez píše v St 08. 06. 2011 v 22:00 -0430:
> More List cleaning up

Great work. I have pushed all patches with the following two changes:

1. 0016-Replace-SbaSelectionList-with-std-vector-sal_In.calc.patch

I replaced
+    if ( !pSelection->empty() && (*pSelection)[0] != -1L )
with
+    if ( !pSelection->empty() && (*pSelection)[0] != -1 )

, see
http://cgit.freedesktop.org/libreoffice/calc/commit/?id=63be842775f2f25fdc47376e0cbd79e2df60f7bc

It was great catch that sal_Int32 was enough => we need not compare
against the long -1.



2. 0017-Replace-List-with-std-vector-ScAddress.calc-pmladek.patch

I replaced

+    long nCount = nNoteStart; 
+        if ( nNoteStart + nCount < nSize) 
+            ScAddress &rPos = aNotePosList[nCount]; 

with

+    long nCount = 0;
+        if ( nNoteStart + nCount < nSize)
+            ScAddress &rPos = aNotePosList[ nNoteStart + nCount ];

, see 
http://cgit.freedesktop.org/libreoffice/calc/commit/?id=dbeb664c3549a00306bd26e2f7b2c3c547b33dfa

By other words, I restored the original value of nCount.

The reason is that nCount is used as the return value. Your change would
modify the behavior. It would start returning the value increased by
nNoteStart. I am not if the return value is used at all but we should
not change the behavior if we are not sure about the consequences.

Of course, your solution looked more optimized. Though, I am sure that 
"nNoteStart + nCount" will not be counted twice. Compilers do powerful
optimizations these days. IMHO, it is perfectly fine to do some things
that will get optimized at runtime but are better readable in the
sources.


Anyway, thanks a lot for contribution!

Best Regards,
Petr