diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2017-07-20 12:41:28 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2017-07-20 15:09:20 +0200 |
commit | 85f08e3e34bea01456eaf8989ac4f77d3900d5c5 (patch) | |
tree | ed03d7dcb54d1c691aec047710c5e52218aeb23d | |
parent | b1601f7333181a04583612ac3b73e68bd7842c88 (diff) |
remove some "optimisation" insanity in ScriptEventContainer
I can only imagine the weird bugs that must have periodically resulted
when we had a hash value collision.
In the process, fix hasElements() to return a useful value
Change-Id: I1d9a052e73332b4b2bbc9c1fd8142c13eb22f1be
Reviewed-on: https://gerrit.libreoffice.org/40226
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
-rw-r--r-- | compilerplugins/clang/unusedfields.readonly.results | 2 | ||||
-rw-r--r-- | include/toolkit/controls/eventcontainer.hxx | 16 | ||||
-rw-r--r-- | toolkit/source/controls/eventcontainer.cxx | 56 |
3 files changed, 20 insertions, 54 deletions
diff --git a/compilerplugins/clang/unusedfields.readonly.results b/compilerplugins/clang/unusedfields.readonly.results index f36b53dea1cb..4dbd91675443 100644 --- a/compilerplugins/clang/unusedfields.readonly.results +++ b/compilerplugins/clang/unusedfields.readonly.results @@ -686,8 +686,6 @@ include/test/sheet/xdatapilottable.hxx:32 apitest::XDataPilotTable xCellForCheck css::uno::Reference<css::table::XCell> include/test/sheet/xnamedranges.hxx:38 apitest::XNamedRanges xSheet css::uno::Reference<css::sheet::XSpreadsheet> -include/toolkit/controls/eventcontainer.hxx:52 - toolkit::ScriptEventContainer mnElementCount sal_Int32 include/tools/inetmime.hxx:66 INetContentTypeParameter m_bConverted _Bool include/tools/multisel.hxx:43 diff --git a/include/toolkit/controls/eventcontainer.hxx b/include/toolkit/controls/eventcontainer.hxx index aba5ce9f3aa4..859b06ee45a3 100644 --- a/include/toolkit/controls/eventcontainer.hxx +++ b/include/toolkit/controls/eventcontainer.hxx @@ -32,24 +32,12 @@ namespace toolkit { -// Hashtable to optimize -typedef std::unordered_map -< - OUString, - sal_Int32, - OUStringHash -> -NameContainerNameMap; - - class ScriptEventContainer : public ::cppu::WeakImplHelper< css::container::XNameContainer, css::container::XContainer > { - NameContainerNameMap mHashMap; - css::uno::Sequence< OUString > mNames; - std::vector< css::uno::Any > mValues; - sal_Int32 mnElementCount; + std::unordered_map< OUString, css::uno::Any, OUStringHash> + mHashMap; css::uno::Type mType; ContainerListenerMultiplexer maContainerListeners; diff --git a/toolkit/source/controls/eventcontainer.cxx b/toolkit/source/controls/eventcontainer.cxx index 3cc2aa802773..f4df038c405a 100644 --- a/toolkit/source/controls/eventcontainer.cxx +++ b/toolkit/source/controls/eventcontainer.cxx @@ -46,33 +46,33 @@ Type ScriptEventContainer::getElementType() sal_Bool ScriptEventContainer::hasElements() { - bool bRet = (mnElementCount > 0); - return bRet; + return !mHashMap.empty(); } // Methods XNameAccess Any ScriptEventContainer::getByName( const OUString& aName ) { - NameContainerNameMap::iterator aIt = mHashMap.find( aName ); + auto aIt = mHashMap.find( aName ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } - sal_Int32 iHashResult = (*aIt).second; - Any aRetAny = mValues[ iHashResult ]; - return aRetAny; + return aIt->second; } Sequence< OUString > ScriptEventContainer::getElementNames() { - return mNames; + Sequence<OUString> aRet(mHashMap.size()); + int i = 0; + for (auto const & pair : mHashMap) + aRet[i++] = pair.first; + return aRet; } sal_Bool ScriptEventContainer::hasByName( const OUString& aName ) { - NameContainerNameMap::iterator aIt = mHashMap.find( aName ); - bool bRet = ( aIt != mHashMap.end() ); - return bRet; + auto aIt = mHashMap.find( aName ); + return aIt != mHashMap.end(); } @@ -83,14 +83,13 @@ void ScriptEventContainer::replaceByName( const OUString& aName, const Any& aEle if( mType != aAnyType ) throw IllegalArgumentException(); - NameContainerNameMap::iterator aIt = mHashMap.find( aName ); + auto aIt = mHashMap.find( aName ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } - sal_Int32 iHashResult = (*aIt).second; - Any aOldElement = mValues[ iHashResult ]; - mValues[ iHashResult ] = aElement; + Any aOldElement = aIt->second; + aIt->second = aElement; // Fire event ContainerEvent aEvent; @@ -109,18 +108,13 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem if( mType != aAnyType ) throw IllegalArgumentException(); - NameContainerNameMap::iterator aIt = mHashMap.find( aName ); + auto aIt = mHashMap.find( aName ); if( aIt != mHashMap.end() ) { throw ElementExistException(); } - sal_Int32 nCount = mNames.getLength(); - mNames.realloc( nCount + 1 ); - mValues.resize( nCount + 1 ); - mNames.getArray()[ nCount ] = aName; - mValues[ nCount ] = aElement; - mHashMap[ aName ] = nCount; + mHashMap[ aName ] = aElement; // Fire event ContainerEvent aEvent; @@ -132,33 +126,20 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem void ScriptEventContainer::removeByName( const OUString& Name ) { - NameContainerNameMap::iterator aIt = mHashMap.find( Name ); + auto aIt = mHashMap.find( Name ); if( aIt == mHashMap.end() ) { throw NoSuchElementException(); } - sal_Int32 iHashResult = (*aIt).second; - Any aOldElement = mValues[ iHashResult ]; - // Fire event ContainerEvent aEvent; aEvent.Source = *this; - aEvent.Element = aOldElement; + aEvent.Element = aIt->second; aEvent.Accessor <<= Name; maContainerListeners.elementRemoved( aEvent ); mHashMap.erase( aIt ); - sal_Int32 iLast = mNames.getLength() - 1; - if( iLast != iHashResult ) - { - OUString* pNames = mNames.getArray(); - pNames[ iHashResult ] = pNames[ iLast ]; - mValues[ iHashResult ] = mValues[ iLast ]; - mHashMap[ pNames[ iHashResult ] ] = iHashResult; - } - mNames.realloc( iLast ); - mValues.resize( iLast ); } // Methods XContainer @@ -174,8 +155,7 @@ void ScriptEventContainer::removeContainerListener( const css::uno::Reference< c ScriptEventContainer::ScriptEventContainer() - : mnElementCount( 0 ), - mType( cppu::UnoType<ScriptEventDescriptor>::get() ), + : mType( cppu::UnoType<ScriptEventDescriptor>::get() ), maContainerListeners( *this ) { } |