[Libreoffice] [PATCH] Get rid of SvULongs in calc

Maciej Rumianowski maciej.rumianowski at gmail.com
Tue Jul 19 02:06:49 PDT 2011


Hi Kohei,
Thanks for review!

> First, for your svl patch:
> 
> 1) in SfxIntegerListItem::GetList(), you do
> 
> +        rList.insert( aIt, m_aList[n] );
> 
> but you can simply call push_back here to append new data to the end of
> the vector.  Also, let's use pre-increment on iterators in the "for"
> loop there i.e. instead of doing aIt++, do ++aIt.  Post-increment on
> iterators is generally more expensive than pre-increment, so it's best
> to stick with pre-increments.

Done, and with push_back we don't need iterator.

> 2) Your new constructors take arrays of different integer type; one
> takes std::vector<sal_uLong> while the other takes
> uno::Sequence<sal_Int32>.  Let's just stick with one type and use
> sal_Int32 in both cases since that's the integer type that this class
> uses internally.

Done and checked in other places to use right type.

> And for your sc patch:
> 
> 3) In ScTabViewShell::Execute() you do
> 
> -            SvULongs aIndexList( 4, 4 );
> +            ::std::vector < sal_uLong > aIndexList( 4, 4 );
> 
> but this actually changes the behavior of the code.  The arguments to
> SvULong's constructor controls how the internal array is laid out during
> initialization but it doesn't populate the container with data.
> Vector's ctor arguments OTOH populate the container with data.  So, your
> new code initializes aIndexList with 4 elements of value 4, which is not
> what the code intends to do.  You can simply do without passing any
> arguments to aIndexList there.

Done, Now I understand the SvULong's constructor. Also changed type to
sal_uInt32.

> 4) Last but not least, please state that you are submitting your patches
> under LGPLv3+/MPL 1.1.

Of course and future also.

Best Regards,
	Maciej



More information about the LibreOffice mailing list