ARROW-2597: [Plasma] remove UniqueIDHasher#2059
ARROW-2597: [Plasma] remove UniqueIDHasher#2059zhijunfu wants to merge 2 commits intoapache:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2059 +/- ##
==========================================
- Coverage 87.44% 86.26% -1.18%
==========================================
Files 189 230 +41
Lines 29376 40159 +10783
==========================================
+ Hits 25687 34643 +8956
- Misses 3689 5516 +1827
Continue to review full report at Codecov.
|
cpp/src/plasma/common.h
Outdated
| }; | ||
|
|
||
| template <> | ||
| struct hash<const ::plasma::UniqueID> { |
There was a problem hiding this comment.
Is it really necessary to have the two hash<> specializations (const and non-const)?
There was a problem hiding this comment.
Thanks for the comment. The const version is mainly for container declarations like below, without const this would not compile. This kind of usage is rare though:)
std::unordered_map<const UniqueID, int> s;
There was a problem hiding this comment.
Ok, I don't think such use makes much sense. See https://stackoverflow.com/questions/17638154/what-is-difference-between-const-and-non-const-key
("map keys are semantically immutable" and also "some implementations re-use map nodes; under those implementations, attempting to use a const key simply won't work")
There was a problem hiding this comment.
@zhijunfu I think it makes a lot of sense to remove it for now, if there is a use case in the future, we can always re-add it. What do you think?
4 participants
Replace UniqueIDHasher with std::hash so that STL containers with ObjectID doesn't need to specify the compare function. This has already been done for Ray, this change applies it to Plasma.