[Libreoffice] [PUSHED][PATCH] eliminate SvUShorts type

Caolán McNamara caolanm at redhat.com
Tue Nov 1 05:30:54 PDT 2011


On Sat, 2011-10-29 at 19:33 +0200, Daniel Di Marco wrote:
> * SvSUhorts.GetData(): I replaced calls of vec.GetData() with 
> &(*vec.begin()), e.g. in editeng/source/rtf/rtfitem.cxx:

I'm kind of a fan of &vec[0] over &(*vec.begin())

> * sorting: qsort -> std::sort, e.g. in cui/source/dialogs/iconcdlg.cxx:
> -qsort( (void*)aUS.GetData(), aUS.Count(), sizeof(sal_uInt16), 
> IconcDlgCmpUS_Impl );
> +std::sort( aUS.begin(), aUS.end() );

Yeah, seems perfectly sane.

> * copying: memcpy -> std::copy, e.g. in cui/source/dialogs/iconcdlg.cxx:
> -memcpy(pRanges, aUS.GetData(), sizeof(sal_uInt16) * aUS.Count());
> +std::copy( aUS.begin(), aUS.end(), pRanges );

Looks alright, these aren't particularly required to be massively
efficient anyway, so no need to over-think it.

Complete patch seems ok to me, though it can get a little tricky to keep
old and new in my head at the same time, so hopefully I didn't cock
anything up. Pushed now, thanks for these.

So, that's finally an end to SvUShorts, gone, dead as a parrot, pretty
cool.

FWIW, in an eye towards being able to blow away additional entire
families of these old-school macros, I think there might only be one or
two users of SvStringsISortDtor in inc/svl/svstdarr.hxx and if not that
specific one, I think at least one of those macro-based String things in
there is only used in one or two places.

C.

As an aside, vector::at checks that the argument is within size() and
throws if it isn't, so when we're in a loop where we know the size of
the vector it's not worth using the slower at() vs unchecked operator[]
IMO e.g.

const sal_uInt16 nCount = rIndexArray.size();
for( sal_uInt16 nIndex = 0; nIndex < nCount; nIndex++ )
{
-    const sal_uInt16 nElement = rIndexArray.GetObject( nIndex );
+    const sal_uInt16 nElement = rIndexArray.at( nIndex );
}

I'd just do...

+    const sal_uInt16 nElement = rIndexArray[nIndex];



More information about the LibreOffice mailing list