[Libreoffice] [PATCH] Easyhack fdo#38831 - Convert some SvStrings to std::vector

[Eike Rathke]

Hi Brad,

On Wednesday, 2011-12-28 16:47:17 +1300, Brad Sowden wrote:

> Any comments on the size_t changes/best practice welcomed.

> diff --git a/sw/source/ui/dochdl/gloshdl.cxx b/sw/source/ui/dochdl/gloshdl.cxx
> @@ -138,13 +136,13 @@ void SwGlossaryHdl::SetCurGroup(const String &rGrp, sal_Bool bApi, sal_Bool bAlw
>              String sCurBase = aTemp.getBase();
>              aTemp.removeSegment();
>              const String sCurEntryPath = aTemp.GetMainURL(INetURLObject::NO_DECODE);
> -            const SvStrings* pPathArr = rStatGlossaries.GetPathArray();
> +            const std::vector<String*> *pPathArr = rStatGlossaries.GetPathArray();
>              sal_uInt16 nCurrentPath = USHRT_MAX;
> -            for(sal_uInt16 nPath = 0; nPath < pPathArr->Count(); nPath++)
> +            for( size_t nPath = 0; nPath < pPathArr->size(); nPath++ )
>              {
>                  if(sCurEntryPath == *(*pPathArr)[nPath])
>                  {
> -                    nCurrentPath = nPath;
> +                    nCurrentPath = static_cast<sal_uInt16>(nPath);
>                      break;
>                  }
>              }

I have some doubt about the sal_uInt16 nCurrentPath usage there. I'm not
familiar at all with that code and have no idea how the PathArray is
controlled as in how many elements could be added or how it is even
used. The SvStrings container wasn't capable of holding more than 64k
elements and things probably went astray anyway if there were more, but
now the vector at least theoretically could hold more elements, so
down casting size_t nPath to sal_uInt16 might not be correct. Changing
things to size_t may be quite invasive though, and if there's some
mechanism that prevents the PathArray to grow beyond 64k elements it's
not nice but fine ;) to downcast. Else it could be a nasty source of
possible failure.

However, apart from that, the loop over all elements to find an
identical string cries out for a hash_map instead of a vector and use
find instead of a loop. That said without having inspected any
neighborhood. It might as well be that due to other context a vector is
appropriate.

Anyhow, that can be optimized and isn't your fault ;) good patch.

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120102/d7624519/attachment.pgp>