Patch: Fix bug 90222 - Replace ScaList with std container

[Eike Rathke]

Hi Ryan,

Thanks for your patch!

On Wednesday, 2015-07-29 22:45:18 +1200, Ryan McCoskrie wrote:

> (I'm having trouble with logerrit)

That is why? We much prefer patches on gerrit, it eases review and
handling a lot. Using git review might also be an option for you. If for
some reason you *have* to use mail attachments, please use git
format-patch to generate the patch to be attached, which preserves
author and date and commit message, or even use the git send-email
command.

That being said ...

> diff --git a/scaddins/source/datefunc/datefunc.cxx b/scaddins/source/datefunc/datefunc.cxx
>  ScaFuncDataList::ScaFuncDataList( ResMgr& rResMgr ) :
> -    nLast( 0xFFFFFFFF )
> +    nLast( contents.begin() )

This looks odd. I think it should be contents.end() instead. But why
change it to iterator anyway? When using a vector, the last accessed
name could still be remembered as offset into the vector.

>  ScaFuncDataList::~ScaFuncDataList()
>  {
> -    for( ScaFuncData* pFData = First(); pFData; pFData = Next() )
> -        delete pFData;
> +    for( std::vector<ScaFuncData*>::iterator it = contents.begin(); it != contents.end(); ++it )
> +        delete *nLast;

This does not work.. it deletes *nLast multiple times. It should be *it
instead.

> -const ScaFuncData* ScaFuncDataList::Get( const OUString& rProgrammaticName ) const
> +const ScaFuncData* ScaFuncDataList::Get( const OUString& rProgrammaticName )
>  {
>      if( aLastName == rProgrammaticName )
> -        return Get( nLast );
> +        return *nLast;

So this could be  return contents[nLast];  if nLast was still the offset
intead of an iterator.

> diff --git a/scaddins/source/datefunc/datefunc.hxx b/scaddins/source/datefunc/datefunc.hxx
> -class ScaFuncDataList : private ScaList
> +class ScaFuncDataList
>  {
> -    OUString             aLastName;
> -    sal_uInt32                  nLast;
> +    OUString                                    aLastName;
> +    std::vector<ScaFuncData*>::iterator         nLast;

Please adapt the variable name when changing the type, we use the 'n'
prefix for numeric variables, it then should be itLast or some such.
However, as said,  size_t nLast  would still be fine.

> +    std::vector<ScaFuncData*>                   contents;

Maybe naming it vContents would be better.

> -    inline const ScaFuncData*   Get( sal_uInt32 nIndex ) const;
> -    const ScaFuncData*          Get( const OUString& rProgrammaticName ) const;
> +    const ScaFuncData*          Get( const OUString& rProgrammaticName );

Instead of making the function non-const you could also make nLast and
aLastName mutable member variables.

All above the same for the pricing Add-In.

> diff --git a/scaddins/source/pricing/pricing.cxx b/scaddins/source/pricing/pricing.cxx
>  ScaFuncDataList::~ScaFuncDataList()
>  {
> -    for( ScaFuncData* pFData = First(); pFData; pFData = Next() )
> -        delete pFData;
> +    for( std::vector<ScaFuncData*>::iterator it = contents.begin(); it != contents.end();  ++it )
> +        delete *it;

This one correctly deletes *it.

Last but not least:

Apparently we don't have your license statement on file, could you
please send us a blanket statement that you contribute all your past and
future patches under the MPLv2 and LGPLv3+ licenses? Best on the dev
mailing list libreoffice at lists.freedesktop.org so we can link to it from
https://wiki.documentfoundation.org/Development/Developers

Something like this does nicely:

All of my past & future contributions to LibreOffice may be
licensed under the MPLv2/LGPLv3+ dual license.

Best use Subject: <your full name> license statement

Sorry for the inconvenience and thank you for cooperating :-)

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GPG key "ID" 0x65632D3A - 2265 D7F3 A7B0 95CC 3918  630B 6A6C D5B7 6563 2D3A
Better use 64-bit 0x6A6CD5B765632D3A here is why: https://evil32.com/
Care about Free Software, support the FSFE https://fsfe.org/support/?erack
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20150729/a468cb8a/attachment.sig>