[Libreoffice-commits] core.git: include/svl svl/source

[Julien Nabet (via logerrit)]

 include/svl/aeitem.hxx      |    3 +--
 svl/source/items/aeitem.cxx |   14 +++++---------
 2 files changed, 6 insertions(+), 11 deletions(-)

New commits:
commit 72d22380f9356b196e51299bf608c192b6fd6cf7
Author:     Julien Nabet <serval2412 at yahoo.fr>
AuthorDate: Wed Oct 2 23:07:56 2019 +0200
Commit:     Julien Nabet <serval2412 at yahoo.fr>
CommitDate: Thu Oct 3 10:06:28 2019 +0200

    Simplify a bit SfxAllEnumItem (svl)
    
    Make private GetValueByPos which is only used in svl/source/items/aeitem.cxx
    Remove "RemoveValue" method which is only used in InsertValue( sal_uInt16 nValue, const OUString &rValue )
    It allows to call once "GetPosByValue" and the assert is useless since we're in the case:
    "else if ( nPos != USHRT_MAX )"
    
    I think we can more optimize by replacing vector by another container for pValues (type "SfxAllEnumValueArr").
    Indeed, we insert and remove a lot in pValues in a specific position. Vector is not the best container for this
    There's also still the part "//FIXME: Optimisation: use binary search or SortArray" in GetPosByValue_ method
    
    Change-Id: I0cafea70e9d0361ef9a7b345ff770a9b888ef85b
    Reviewed-on: https://gerrit.libreoffice.org/80089
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
    Tested-by: Jenkins
    Reviewed-by: Julien Nabet <serval2412 at yahoo.fr>

diff --git a/include/svl/aeitem.hxx b/include/svl/aeitem.hxx
index 648986825bcf..4c25643cf59d 100644
--- a/include/svl/aeitem.hxx
+++ b/include/svl/aeitem.hxx
@@ -43,6 +43,7 @@ class SVL_DLLPUBLIC SfxAllEnumItem: public SfxAllEnumItem_Base
 
     sal_uInt16              GetPosByValue( sal_uInt16 nValue ) const;
     std::size_t             GetPosByValue_( sal_uInt16 nValue ) const;
+    sal_uInt16              GetValueByPos( sal_uInt16 nPos ) const;
 
 public:
     explicit                SfxAllEnumItem( sal_uInt16 nWhich);
@@ -52,10 +53,8 @@ public:
 
     void                    InsertValue( sal_uInt16 nValue );
     void                    InsertValue( sal_uInt16 nValue, const OUString &rText );
-    void                    RemoveValue( sal_uInt16 nValue );
 
     virtual sal_uInt16      GetValueCount() const override;
-    sal_uInt16              GetValueByPos( sal_uInt16 nPos ) const;
     OUString const &        GetValueTextByPos( sal_uInt16 nPos ) const;
     virtual SfxPoolItem*    Clone( SfxItemPool *pPool = nullptr ) const override;
 };
diff --git a/svl/source/items/aeitem.cxx b/svl/source/items/aeitem.cxx
index 03431b91594a..cafa6fe3a3b7 100644
--- a/svl/source/items/aeitem.cxx
+++ b/svl/source/items/aeitem.cxx
@@ -108,11 +108,14 @@ void SfxAllEnumItem::InsertValue( sal_uInt16 nValue, const OUString &rValue )
     SfxAllEnumValue_Impl aVal;
     aVal.nValue = nValue;
     aVal.aText = rValue;
+    sal_uInt16 nPos = GetPosByValue(nValue);
     if ( !pValues )
         pValues.reset( new SfxAllEnumValueArr );
-    else if ( GetPosByValue( nValue ) != USHRT_MAX )
+    else if ( nPos != USHRT_MAX )
+    {
         // remove when exists
-        RemoveValue( nValue );
+        pValues->erase( pValues->begin() + nPos );
+    }
     // then insert
     pValues->insert(pValues->begin() + GetPosByValue_(nValue), aVal); // FIXME: Duplicates?
 }
@@ -128,11 +131,4 @@ void SfxAllEnumItem::InsertValue( sal_uInt16 nValue )
     pValues->insert(pValues->begin() + GetPosByValue_(nValue), aVal); // FIXME: Duplicates?
 }
 
-void SfxAllEnumItem::RemoveValue( sal_uInt16 nValue )
-{
-    sal_uInt16 nPos = GetPosByValue(nValue);
-    assert(nPos != USHRT_MAX);
-    pValues->erase( pValues->begin() + nPos );
-}
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */