Re: [PATCH V2] more conversion of tools/Table.hxx to std::map in editeng, sc and sw modules

On Tue, Feb 07, 2012 at 05:02:18PM +0200, Noel Grandin wrote:
> Hi
>
> Attached patch converts various places in the editeng, sc and sw
> modules from using tools/table.hxx to using std::map

Why not split it into more patches, preferably one for each replaced
class? It would make review easier (and it would also make revert of
only one of the conversions easier, if it proves necessary ;-)

A couple of comments:

>  SvxFontTable::~SvxFontTable()
>  {
> -    SvxFontItem* pItem = First();
> -    while( pItem )
> -    {
> -        delete pItem;
> -        pItem = Next();
> -    }
> +     for(FontItemMap::iterator it = maFontItemMap.begin(); it != maFontItemMap.end(); ++it)
> +     {
> +        delete it->second;
> +     }
>  }

IMHO it would be more appropriate to use boost::ptr_map here.

> -DECLARE_TABLE( DummyFontTable, SvxFontItem* )
>  
> -class SvxFontTable : public DummyFontTable
> +class SvxFontTable
>  {
>  public:
>              SvxFontTable();
>              ~SvxFontTable();
>  
> -    sal_uLong   GetId( const SvxFontItem& rFont );
> +    sal_uLong    GetId( const SvxFontItem& rFont ) const;
> +     void             Insert( sal_uLong nIdx, SvxFontItem *pFontItem);
> +     int                      Count() const;
> +     SvxFontItem* Get( sal_uLong nIdx );
> +

You add "compatibility" functions here, but replace them by std::map
functions at other places. Are these so widely used it would be
impractical?

> -const com::sun::star::i18n::ForbiddenCharacters* 
> SvxForbiddenCharactersTable::GetForbiddenCharacters( sal_uInt16 nLanguage, sal_Bool bGetDefault ) 
> const
> +const com::sun::star::i18n::ForbiddenCharacters* 
> SvxForbiddenCharactersTable::GetForbiddenCharacters( sal_uInt16 nLanguage, sal_Bool bGetDefault )
>  {
> -    ForbiddenCharactersInfo* pInf = Get( nLanguage );
> +    ForbiddenCharactersInfo* pInf = GetCharInfo( nLanguage );
>      if ( !pInf && bGetDefault && mxMSF.is() )
>      {
> -        const SvxForbiddenCharactersTableImpl *pConstImpl = dynamic_cast<const 
> SvxForbiddenCharactersTableImpl*>(this);
> -        SvxForbiddenCharactersTableImpl* pImpl = 
> const_cast<SvxForbiddenCharactersTableImpl*>(pConstImpl);
> -         pInf = new ForbiddenCharactersInfo;
> -        pImpl->Insert( nLanguage, pInf );
> +        pInf = new ForbiddenCharactersInfo;
> +        maCharInfoMap[ nLanguage ] = pInf;
>  
>          pInf->bTemporary = sal_True;
>          LocaleDataWrapper aWrapper( mxMSF, SvxCreateLocale( nLanguage ) );

This function can remain const if you declare the maCharInfoMap member
mutable. But that is just a nitpick :-)

Thank you for working on this!

D.