-
Notifications
You must be signed in to change notification settings - Fork 767
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
Add new insert_or_assign method to Values #959
Conversation
Already exists as tryInsert ? |
tryInsert doesn't perform an update if the key already exists. upsert will perform the update. |
I’m not in love with this. If we modify Values, I’d prefer to align it more with stl::map, especially the c++11 and c++17 versions. It’s probably better to do a design-review these PRs to the core data structures via an issue that proposes a change, before spending a lot of time. If you disagree strongly, let’s discuss off-line. |
Adding some research I did here on templated operator overloading, thanks to this link. template <typename T>
T& operator[](Key k) {
...
} but the operator call would be ugly Values values;
Pose3 pose;
values[key] = pose; // Compiler error due to template mismatch!!
values.operator[]<Pose3>(pose); // Works, but is more verbose and defeats the purpose. |
Yeah, that’s really ugly. Did you read the entire c++17 interface to standard map though? Is there another function that already exists that does what the imperative operator does? |
I did. There's I guess the cpp standards committee likes verbose function names. |
Yeah. I like shorter names as well, but |
Cool, I'll update the method name. :) |
@dellaert updated |
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.
Nice !
This PR adds a new method
insert_or_assign
in support of #958.Modern databases support theupsert
method which tries to perform an update and, if a key doesn't exist, defaults to an insert.insert_or_assign
is a C++17std::map
method so this method aligns us better with that.This simplifies this common pattern:
to the more concise: