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
pgphXbtlzVz9b.pgp
Description: PGP signature
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
