Re: [Libreoffice] [PATCH] Replace SvULongs with vector and code clean up part 1

Hi Maciej,

On Sunday, 2011-08-07 17:04:49 +0200, Maciej Rumianowski wrote:

> > You're right, the code is "suboptimal" ... in fact IsRemoved_Impl()
> > and
> > IsAdded_Impl() could also return the index of the element found so it
> > wouldn't be needed to be searched again. Want to take care of it? 
>
> Yes :) My proposition of new declaration
>
> > bool IsAdded_Impl( size_t nKey, size_t* nFoundKey );

I suggest

::std::vector<sal_uInt32>::iterator GetAdded_Impl( size_t nKey );

with logically the content of IsAdded_Impl() but using ::std::find()
instead of the awkward loop and returning the position's iterator that
can be used in RemoveFormat(), plus

bool IsAdded_Impl( size_t nKey )
{
    return GetAdded_Impl( nKey) != aAddList.end();
}

for all other places that need only a bool to remain the same.


> I am attaching corrected patch, without this change and without size_t.
> I have casted variables to size_t,

Looks good, with some minor exception, see below, I'll commit with that
change once my build will have finished..

> in next patch I will try to change short to appropriate type and in other places try to use 
> size_t.

Take care not to touch too much, some of the short type seems to evolve
from the list box using that type for position of elements. I think it
doesn't pay to be overly aggressive there.


> @@ -1488,21 +1464,15 @@ String SvxNumberFormatShell::GetFormat4Entry(short nEntry)
>  short SvxNumberFormatShell::GetListPos4Entry(sal_uInt32 nIdx)
>  {
>      short nSelP=SELPOS_NONE;
> -    if( aCurEntryList.Count() <= 0x7fff )
> +
> +    for(size_t i=0; i < aCurEntryList.size(); ++i)
>      {
> -        for(short i=0;i<aCurEntryList.Count();i++)
> +        if(aCurEntryList[i]==nIdx)
>          {
> -            if(aCurEntryList[i]==nIdx)
> -            {
> -                nSelP=i;
> -                break;
> -            }
> +            nSelP=i;
> +            break;
>          }
>      }
> -    else
> -    {
> -        OSL_FAIL("svx::SvxNumberFormatShell::GetListPos4Entry(), list got to large!" );
> -    }
>      return nSelP;
>  }

Here the condition checking for 0x7fff should not be removed. This is
more hypothetical though, as the currency list will never (did I say
never? well, hopefully..) grow beyond that. Clearly the fixed value
should depend on the return type used, so
::std::numeric_limits<short>::max() in this case.

  Eike

-- 
 PGP/OpenPGP/GnuPG encrypted mail preferred in all private communication.
 Key ID: 0x293C05FD - 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD

Attachment: pgpGpb2iODkVv.pgp
Description: PGP signature