From 6933bc3021c6ee106ab8088e8f3c41058fd813a3 Mon Sep 17 00:00:00 2001 From: Mark Olesen <Mark.Olesen@esi-group.com> Date: Mon, 15 May 2017 09:57:25 +0200 Subject: [PATCH] ENH: HashPtrTable remove/erase now include safeguard against end-iterator - This makes the following safe: auto iter = ptrTable.find(unknownKey); ptrTable.erase(iter); - Unmask HashPtrTable::erase(const Key& key) method --- .../test/HashPtrTable/Test-hashPtrTable.C | 43 +++++++++++++++---- .../HashTables/HashPtrTable/HashPtrTable.C | 42 ++++++++++++------ .../HashTables/HashPtrTable/HashPtrTable.H | 9 +++- .../HashTables/HashTable/HashTable.C | 3 +- .../HashTables/HashTable/HashTable.H | 10 ++++- 5 files changed, 80 insertions(+), 27 deletions(-) diff --git a/applications/test/HashPtrTable/Test-hashPtrTable.C b/applications/test/HashPtrTable/Test-hashPtrTable.C index 5133d784b18..e62d5968698 100644 --- a/applications/test/HashPtrTable/Test-hashPtrTable.C +++ b/applications/test/HashPtrTable/Test-hashPtrTable.C @@ -36,14 +36,9 @@ void printTable(const HashPtrTable<T>& table) { Info<< table.size() << nl << "(" << nl; - for - ( - typename HashPtrTable<T>::const_iterator iter = table.cbegin(); - iter != table.cend(); - ++iter - ) + forAllConstIters(table, iter) { - const T* ptr = *iter; + const T* ptr = iter.object(); Info<< iter.key() << " = "; if (ptr) { @@ -57,6 +52,22 @@ void printTable(const HashPtrTable<T>& table) } Info<< ")" << endl; + + // Values only, with for-range + Info<< "values ("; + for (auto val : table) + { + Info<< ' '; + if (val) + { + Info<< *val; + } + else + { + Info<< "nullptr"; + } + } + Info<< " )" << nl; } @@ -68,7 +79,9 @@ int main() HashPtrTable<double> myTable; myTable.insert("abc", new double(42.1)); myTable.insert("def", nullptr); - myTable.insert("ghi", new double(3.14159)); + myTable.insert("pi", new double(3.14159)); + myTable.insert("natlog", new double(2.718282)); + myTable.insert("sqrt2", new double(1.414214)); // Info<< myTable << endl; printTable(myTable); @@ -79,8 +92,20 @@ int main() printTable(copy); Info<< copy << endl; + Info<<"\nerase some existing and non-existing entries" << nl; + + auto iter = myTable.find("pi"); + myTable.erase(iter); + + iter = myTable.find("unknownKey"); + myTable.erase(iter); + + myTable.erase("abc"); + myTable.erase("unknownKey"); + + printTable(myTable); + return 0; } - // ************************************************************************* // diff --git a/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.C b/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.C index 4143d25d852..9370dd26edf 100644 --- a/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.C +++ b/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.C @@ -72,30 +72,44 @@ Foam::HashPtrTable<T, Key, Hash>::~HashPtrTable() template<class T, class Key, class Hash> T* Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter) { - T* ptr = iter.object(); - this->parent_type::erase(iter); - return ptr; + if (iter.found()) + { + T* ptr = iter.object(); + this->parent_type::erase(iter); + return ptr; + } + + return nullptr; } template<class T, class Key, class Hash> bool Foam::HashPtrTable<T, Key, Hash>::erase(iterator& iter) { - T* ptr = iter.object(); - - if (this->parent_type::erase(iter)) + if (iter.found()) { - if (ptr) + T* ptr = iter.object(); + + if (this->parent_type::erase(iter)) { - delete ptr; - } + if (ptr) + { + delete ptr; + } - return true; - } - else - { - return false; + return true; + } } + + return false; +} + + +template<class T, class Key, class Hash> +bool Foam::HashPtrTable<T, Key, Hash>::erase(const Key& key) +{ + auto iter = this->find(key); + return this->erase(iter); } diff --git a/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.H b/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.H index 7cdf3743e31..f83c04e3139 100644 --- a/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.H +++ b/src/OpenFOAM/containers/HashTables/HashPtrTable/HashPtrTable.H @@ -116,12 +116,17 @@ public: // Edit - //- Remove and return the pointer specified by given iterator + //- Remove and return the pointer specified by given iterator. + // Includes a safeguard against the end-iterator. T* remove(iterator& iter); - //- Erase an hashedEntry specified by given iterator + //- Erase an entry specified by given iterator + // Includes a safeguard against the end-iterator. bool erase(iterator& iter); + //- Erase an entry specified by the given key + bool erase(const Key& key); + //- Clear all entries from table void clear(); diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C index 5916a753bf7..276f617dba1 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.C @@ -411,7 +411,8 @@ bool Foam::HashTable<T, Key, Hash>::erase(const iterator& iter) template<class T, class Key, class Hash> bool Foam::HashTable<T, Key, Hash>::erase(const Key& key) { - return erase(find(key)); + auto iter = find(key); + return erase(iter); } diff --git a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H index 3ba008d5575..9eef011d10e 100644 --- a/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H +++ b/src/OpenFOAM/containers/HashTables/HashTable/HashTable.H @@ -349,7 +349,15 @@ public: inline bool set(const Key& key, const T& obj); //- Erase an entry specified by given iterator - // This invalidates the iterator until the next ++ operation + // This invalidates the iterator until the next ++ operation. + // + // Includes a safeguard against the end-iterator such that the + // following is safe: + // \code + // auto iter = table.find(unknownKey); + // table.erase(iter); + // \endcode + // which is what \code table.erase(unknownKey) \endcode does anyhow bool erase(const iterator& iter); //- Erase an entry specified by the given key -- GitLab