Skip to content
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

Implements issue 1491 (check for unique keys) #1541

Closed
wants to merge 9 commits into from
Closed

Conversation

lemire
Copy link
Member

@lemire lemire commented Apr 7, 2021

Some users may want to check that the keys are unique. For those users, we may provide find_unique_field. It works like find_unordered_field but scans keys in a loop to check for uniqueness. We hope that users would not rely on it within performance critical code since it is potentially quite expensive. However, it is nice feature to have around.

Fixes #1491

@lemire lemire changed the title Implements issue 1491 Implements issue 1491 (check for unique keys) Apr 23, 2021
@jkeiser
Copy link
Member

jkeiser commented Apr 30, 2021

Hmm. I don't have an issue with this at all, but the more methods like this we gather, the more we should think about fixing it systemically by having the ability to index the fields or do multiple in one pass.

@lemire
Copy link
Member Author

lemire commented Apr 30, 2021

@jkeiser We do not have to merge this, we never got a request of this type... but I only implemented it as a precaution, and a proof of concept. We certainly do not have to merge it for release 1.0.

There would be other ways to handle this...

  1. Build a single-pass matcher... like this...
auto [value1, value2, value3] = object.get("key1", "key2", "key3"); 

It would always fully scan the object, and might check for duplicates as it does so. That could possibly be quite performant.

  1. Don't worry about duplicates but have the object iterator track the searched keys... as you encounter new keys, check whether you have had a request for them in the past... so... given... {"x":1, "x":2, "y":2}, you'd go...
auto x = object["x"];
auto y = object["y"]; // an error here would happen

Neither of these alternatives is super pleasant.

@jkeiser
Copy link
Member

jkeiser commented Apr 30, 2021

I could imagine the single-pass matcher working, especially if each of them returns a fork() so you could iterate them independently. I could also imagine some kind of object.get_map() which returns map<std::string_view, value> or whatever type a forked value is ...

@lemire
Copy link
Member Author

lemire commented Apr 30, 2021

I am happy to let this PR sit for a while longer.

@lemire
Copy link
Member Author

lemire commented May 27, 2021

I do not like this PR after all. Closing.

@lemire lemire closed this May 27, 2021
@lemire lemire deleted the dlemire/issue1491 branch July 7, 2021 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complement find_field() with find_unique_field()
2 participants