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

[slot_map] Can you get a key using an iterator? #141

Closed
p-groarke opened this issue Dec 11, 2018 · 5 comments
Closed

[slot_map] Can you get a key using an iterator? #141

p-groarke opened this issue Dec 11, 2018 · 5 comments

Comments

@p-groarke
Copy link

p-groarke commented Dec 11, 2018

I'm trying to find the keys of stored objects that match a given predicate. I can't see any api to get a key from an iterator, but I may be missing something.

Is it possible to get a key using an iterator in slot_map?

@p-groarke
Copy link
Author

OK I believe that is provided by slot_iter_from_value_iter, it is simply marked private.

@Quuxplusone
Copy link
Contributor

This, like #131, sounds like a legitimate design issue. @Masstronaut, are you still interested in slot_map? Are these features good ideas, or did you consider them and then exclude them from the slot_map proposal for reasons that it would be worthwhile to recap here?

@p-groarke: If the functionality you think-you-need is there-but-private, then this issue should still be open, right? Either we should make the functionality public, or we should explain why you don't really need it, or we should explain why if you do need it you shouldn't be using slot_map.

My kneejerk reaction is that we should make it public, which means bikeshedding the name/API. Like,

slot_map<K, V> sm;
std::vector<V>::iterator it = sm.begin();
K k = sm.key(it);  // new functionality -- is this a good API?
assert(sm.find(k) == it);

@Quuxplusone Quuxplusone reopened this Dec 11, 2018
@p-groarke
Copy link
Author

@Quuxplusone Fair point, until we get an answer from the author and windows CI, my hands are a little tied. I have many features I'd like to add. I'm happy to send a PR for this one if the original author is on board / doesn't have objections.

@Quuxplusone
Copy link
Contributor

@p-groarke: If you have "many features" you'd like to add, I'd recommend forking the repo, adding all the features in a branch, and then submitting the whole thing as a big patch so we can see how all the new things work together. Bugfixes = incremental, design work = holistic. :) (#135 is certainly related, if we're doing design work.)

FWIW, I haven't seen any comments on this repo from @Masstronaut (Allan Deutsch) for a long time. I myself am the original author of this repo's "slot_map.h", which I did just by slavishly following Allan's P0661R0. Allan has his own slot_map at https://github.com/Masstronaut/slot_array .

I see there is also https://github.com/twiggler/slotmap . I don't know anything about it except that it has a lot of heavy dependencies: std::random_device, Boost.Integer, Boost.Hana,...

@p-groarke
Copy link
Author

p-groarke commented Dec 11, 2018

@Quuxplusone Sounds good to me. Will continue to gain some experience with the container in the meantime. I personally really like the name you proposed sm.key(it). It is short and to-the-point. I have some general questions, would you rather I open another issue or email you with them?

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

No branches or pull requests

2 participants