Re: [Libreoffice] [PATCH 1/8][PUSHED] Easyhack fdo#38831 remove SvStrings

On Thursday 29 of December 2011, Brad Sowden wrote:
> Hi,
>
> See attached.
>
> Note 1
> ------
> I assume casting from size_t to sal_uInt16 is reasonable in the
> following scenario (I didn't want to update all callers in this patch)?
>
>   sal_uInt16 SwGlossaries::GetGroupCnt()
>   {
> -    return  GetNameList()->Count();
> +    return static_cast<sal_uInt16>(GetNameList()->size());
>   }

 The whole class seems to use sal_uInt16 there, so this at least doesn't make 
things worse.

> Note 2
> ------
> In 2 functions I made the following change. Is this ok or should I also
> call reserve(16) to be safe?
>
> -            SvStrings aFiles( 16, 16 );
> +            std::vector<String*> aFiles;

 Calling reserve() is just avoiding reallocations, so unless something in the 
code relies on the fact that there will be no reallocations (i.e. addresses 
remain the same), which does not seem to be the case, this is just an 
optimization. The 16 looks a bit like a magic number anyway.


 You also removed the check for m_pGlosArr being NULL in the code below:
SwGlossaries::~SwGlossaries()
{
- sal_uInt16 nCount = m_pGlosArr? m_pGlosArr->Count() : 0;

 Fixed that, pushed.

-- 
 Lubos Lunak
 l.lunak@suse.cz