Improve HashMap/Set situation #137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
floriankramer
approved these changes
Nov 15, 2018
There was a problem hiding this comment.
The changes look good to me, apart from one or two minor things. Especially the comparator when creating the patterns vector looks a lot cleaner.
How would you suggest we go about switching out the current usage of std::unordered_map with HashMap? I'd suggest creating one pr that replaces all usages of unordered_map with hash map, but depending on the amount of other pending changes that might be unfeasible due to the amount of files changed.
test/HashMapTest.cpp
Outdated
| ASSERT_EQ(maptwo["bar"], 41); | ||
| }; | ||
|
|
||
| int main(int argc, char** argv) { |
There was a problem hiding this comment.
The other tests link against 'gtest_main' for a main method.
|
I've changed all possible uses in master using |
This starts work on issue #136 by using inheritance plus using declerations instead of explicit method wrapping. This also fixes the broken insert() method which couldn't actually be used as no call to it could ever compile. Also adds rudimentary sanity tests (the underlying implementations should be well tested already).
This makes IndexTest independent of the order of the entries in the patternCounts hash map
Used `sed` to refactor `std::unordered_map` except one where `ad_utility::HashMap` currently doesn't work because of missing `DELETED_KEY` or `iterator`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This starts work on issue #136 by using inheritance plus using
declerations instead of explicit method wrapping. This also fixes the
broken insert() method which couldn't actually be used as no call to it
could ever compile.
Also adds rudimentary sanity tests (the underlying implementations
should be well tested already).