Simplify NameContainer

Having a map, two vectors, and a counter for what is a job for a single
map is a bit too much.

Change-Id: I50d29f03a2463e6f63ed8e645c36b4c77aa1413a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177604
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

basic/source/inc/namecont.hxx

@@ -66,12 +66,7 @@ typedef ::comphelper::WeakImplHelper<
 
 class NameContainer final : public NameContainer_BASE
 {
-    typedef std::unordered_map < OUString, sal_Int32 > NameContainerNameMap;
+    std::unordered_map<OUString, css::uno::Any> maMap;
-
-    NameContainerNameMap mHashMap;
-    std::vector< OUString > mNames;
-    std::vector< css::uno::Any > mValues;
-    sal_Int32 mnElementCount;
 
     css::uno::Type mType;
     css::uno::XInterface* mpxEventSource;
@@ -81,20 +76,13 @@ class NameContainer final : public NameContainer_BASE
 
 public:
     NameContainer( const css::uno::Type& rType )
-        : mnElementCount( 0 )
+        : mType( rType )
-        , mType( rType )
         , mpxEventSource( nullptr )
     {}
 
     void setEventSource( css::uno::XInterface* pxEventSource )
         { mpxEventSource = pxEventSource; }
 
-    /// @throws css::lang::IllegalArgumentException
-    /// @throws css::container::ElementExistException
-    /// @throws css::lang::WrappedTargetException
-    /// @throws css::uno::RuntimeException
-    void insertCheck(const OUString& aName, const css::uno::Any& aElement);
-
     /// @throws css::lang::IllegalArgumentException
     /// @throws css::lang::WrappedTargetException
     /// @throws css::uno::RuntimeException

basic/source/uno/namecont.cxx

@@ -107,33 +107,28 @@ Type NameContainer::getElementType()
 
 sal_Bool NameContainer::hasElements()
 {
-    bool bRet = (mnElementCount > 0);
+    return !maMap.empty();
-    return bRet;
 }
 
 // Methods XNameAccess
 Any NameContainer::getByName( const OUString& aName )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = maMap.find(aName);
-    if( aIt == mHashMap.end() )
+    if (aIt == maMap.end())
     {
-        throw NoSuchElementException();
+        throw NoSuchElementException(aName);
     }
-    sal_Int32 iHashResult = (*aIt).second;
+    return aIt->second;
-    Any aRetAny = mValues[ iHashResult ];
-    return aRetAny;
 }
 
 Sequence< OUString > NameContainer::getElementNames()
 {
-    return comphelper::containerToSequence(mNames);
+    return comphelper::mapKeysToSequence(maMap);
 }
 
 sal_Bool NameContainer::hasByName( const OUString& aName )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    return maMap.contains(aName);
-    bool bRet = ( aIt != mHashMap.end() );
-    return bRet;
 }
 
 
@@ -145,15 +140,13 @@ void NameContainer::replaceByName( const OUString& aName, const Any& aElement )
     {
         throw IllegalArgumentException(u"types do not match"_ustr, getXWeak(), 2);
     }
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = maMap.find(aName);
-    if( aIt == mHashMap.end() )
+    if (aIt == maMap.end())
     {
-        throw NoSuchElementException();
+        throw NoSuchElementException(aName);
     }
-    sal_Int32 iHashResult = (*aIt).second;
+    Any aOldElement = aIt->second;
-    Any aOldElement = mValues[ iHashResult ];
+    aIt->second = aElement;
-    mValues[ iHashResult ] = aElement;
-
 
     std::unique_lock aGuard(m_aMutex);
 
@@ -181,16 +174,6 @@ void NameContainer::replaceByName( const OUString& aName, const Any& aElement )
     }
 }
 
-void NameContainer::insertCheck(const OUString& aName, const Any& aElement)
-{
-    NameContainerNameMap::iterator aIt = mHashMap.find(aName);
-    if( aIt != mHashMap.end() )
-    {
-        throw ElementExistException();
-    }
-    insertNoCheck(aName, aElement);
-}
-
 void NameContainer::insertNoCheck(const OUString& aName, const Any& aElement)
 {
     const Type& aAnyType = aElement.getValueType();
@@ -199,12 +182,7 @@ void NameContainer::insertNoCheck(const OUString& aName, const Any& aElement)
         throw IllegalArgumentException(u"types do not match"_ustr, getXWeak(), 2);
     }
 
-    sal_Int32 nCount = mNames.size();
+    maMap[aName] = aElement;
-    mNames.push_back( aName );
-    mValues.push_back( aElement );
-
-    mHashMap[ aName ] = nCount;
-    mnElementCount++;
 
     std::unique_lock aGuard(m_aMutex);
 
@@ -234,31 +212,21 @@ void NameContainer::insertNoCheck(const OUString& aName, const Any& aElement)
 // Methods XNameContainer
 void NameContainer::insertByName( const OUString& aName, const Any& aElement )
 {
-    insertCheck(aName, aElement);
+    if (hasByName(aName))
+        throw ElementExistException(aName);
+    insertNoCheck(aName, aElement);
 }
 
 void NameContainer::removeByName( const OUString& aName )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = maMap.find(aName);
-    if( aIt == mHashMap.end() )
+    if (aIt == maMap.end())
     {
-        OUString sMessage = "\"" + aName + "\" not found";
+        throw NoSuchElementException("\"" + aName + "\" not found");
-        throw NoSuchElementException(sMessage);
     }
 
-    sal_Int32 iHashResult = (*aIt).second;
+    Any aOldElement = aIt->second;
-    Any aOldElement = mValues[ iHashResult ];
+    maMap.erase(aIt);
-    mHashMap.erase( aIt );
-    sal_Int32 iLast = mNames.size() - 1;
-    if( iLast != iHashResult )
-    {
-        mNames[ iHashResult ] = mNames[ iLast ];
-        mValues[ iHashResult ] = mValues[ iLast ];
-        mHashMap[ mNames[ iHashResult ] ] = iHashResult;
-    }
-    mNames.resize( iLast );
-    mValues.resize( iLast );
-    mnElementCount--;
 
     std::unique_lock aGuard(m_aMutex);