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