-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace map count/insert w/emplace in instantx.cpp #2165
Conversation
This feels like an optimization, something about: ``` if map.count(key) return false else insert(pair) return true ``` ... just feels icky. Plus, `vote.GetMasternodeOutpoint()` is only called once. The previous version might be slightly more readable, however.
src/instantx.cpp
Outdated
return false; | ||
mapMasternodeVotes.insert(std::make_pair(vote.GetMasternodeOutpoint(), vote)); | ||
return true; | ||
return mapMasternodeVotes.emplace(std::make_pair(vote.GetMasternodeOutpoint(), vote)).second; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does the .second
do in this instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emplace
is a method defined on std::map
, so I will point to the documentation there (in case you're interested):
https://en.cppreference.com/w/cpp/container/map/emplace
Return value
Returns a pair consisting of an iterator to the inserted element, or the already-existing element if no insertion happened, and a bool denoting whether the insertion took place. True for Insertion, False for No Insertion.
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, that makes more sense. I guess I missed the first part of the return doc when I read it a few minutes ago. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for make_pair
here with emplace
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, can be simplified a bit more though, see inline comment ;)
Also, maybe apply a similar fix to AcceptLockRequest
/RejectLockRequest
while at it?
src/instantx.cpp
Outdated
return false; | ||
mapMasternodeVotes.insert(std::make_pair(vote.GetMasternodeOutpoint(), vote)); | ||
return true; | ||
return mapMasternodeVotes.emplace(std::make_pair(vote.GetMasternodeOutpoint(), vote)).second; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for make_pair
here with emplace
.
You're right, good catch.
These functions just call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, ignore that part :) That should probably be in some another, more general PR.
utACK
@UdjinM6 I fully agree: nmarley@03af44a ;-) Would like to get that larger PR in after iterator cleanup if possible, so easier to merge w/o conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
* replace map count/insert w/emplace in instantx.cpp This feels like an optimization, something about: ``` if map.count(key) return false else insert(pair) return true ``` ... just feels icky. Plus, `vote.GetMasternodeOutpoint()` is only called once. The previous version might be slightly more readable, however. * use std::pair template constructor shortcut
This feels like an optimization, something about:
... just feels icky. Plus,
vote.GetMasternodeOutpoint()
is only calledonce. The previous version might be slightly more readable, however.