From ea2a97438643a440e192dbe30d03988da65c6b18 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Sun, 1 Dec 2024 14:25:12 +0500 Subject: [PATCH] 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 --- basic/source/inc/namecont.hxx | 16 +------- basic/source/uno/namecont.cxx | 74 ++++++++++------------------------- 2 files changed, 23 insertions(+), 67 deletions(-) diff --git a/basic/source/inc/namecont.hxx b/basic/source/inc/namecont.hxx index 0fea0a27a550..e466e714cae0 100644 --- a/basic/source/inc/namecont.hxx +++ b/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; - - NameContainerNameMap mHashMap; - std::vector< OUString > mNames; - std::vector< css::uno::Any > mValues; - sal_Int32 mnElementCount; + std::unordered_map maMap; 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 diff --git a/basic/source/uno/namecont.cxx b/basic/source/uno/namecont.cxx index 94c4f9a2dd39..714a94e14fdb 100644 --- a/basic/source/uno/namecont.cxx +++ b/basic/source/uno/namecont.cxx @@ -107,33 +107,28 @@ Type NameContainer::getElementType() sal_Bool NameContainer::hasElements() { - bool bRet = (mnElementCount > 0); - return bRet; + return !maMap.empty(); } // Methods XNameAccess Any NameContainer::getByName( const OUString& aName ) { - NameContainerNameMap::iterator aIt = mHashMap.find( aName ); - if( aIt == mHashMap.end() ) + auto aIt = maMap.find(aName); + if (aIt == maMap.end()) { - throw NoSuchElementException(); + throw NoSuchElementException(aName); } - sal_Int32 iHashResult = (*aIt).second; - Any aRetAny = mValues[ iHashResult ]; - return aRetAny; + return aIt->second; } 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 ); - bool bRet = ( aIt != mHashMap.end() ); - return bRet; + return maMap.contains(aName); } @@ -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 ); - if( aIt == mHashMap.end() ) + auto aIt = maMap.find(aName); + if (aIt == maMap.end()) { - throw NoSuchElementException(); + throw NoSuchElementException(aName); } - sal_Int32 iHashResult = (*aIt).second; - Any aOldElement = mValues[ iHashResult ]; - mValues[ iHashResult ] = aElement; - + Any aOldElement = aIt->second; + aIt->second = 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(); - mNames.push_back( aName ); - mValues.push_back( aElement ); - - mHashMap[ aName ] = nCount; - mnElementCount++; + maMap[aName] = aElement; 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 ); - if( aIt == mHashMap.end() ) + auto aIt = maMap.find(aName); + if (aIt == maMap.end()) { - OUString sMessage = "\"" + aName + "\" not found"; - throw NoSuchElementException(sMessage); + throw NoSuchElementException("\"" + aName + "\" not found"); } - sal_Int32 iHashResult = (*aIt).second; - Any aOldElement = mValues[ iHashResult ]; - 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--; + Any aOldElement = aIt->second; + maMap.erase(aIt); std::unique_lock aGuard(m_aMutex);