-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat(map): Add GetNextKey and GetValueAndDeleteKey #411
Conversation
@geyslan Hi, geyslan Support map apis in todo list, please review when you are free, thanks lot. |
1592a92
to
8a5061f
Compare
@chentao-kernel I'm baking the next version. Please tell me if you'll have time to finish this so it can be part of it. 👍🏼 |
Yes, i can finish it |
8a5061f
to
72f0885
Compare
@geyslan update it, please check again |
Add GetNextKey(), GetValueAndDeleteKey() and GetValueAndDeleteKeyFlags() to the map package. Signed-off-by: Tao Chen <[email protected]> Co-authored-by: Geyslan Gregório <[email protected]>
@chentao-kernel I raised this chentao-kernel#3. |
Add GetNextKey(), GetValueAndDeleteKey() and GetValueAndDeleteKeyFlags() to the map package. Signed-off-by: Tao Chen <[email protected]> Co-authored-by: Geyslan Gregório <[email protected]>
72f0885
to
03d86e3
Compare
Dylane/key
@geyslan hi, geyslan, thank you for your help, it was mergered. |
@chentao-kernel, I was rethinking this API and realised that the initial TODO comment led to an erroneous GetNextKey signature as it is not performant (always allocating a buffer to store the fetched key value). This idea was brought up because in the current codebase we have some methods that work as helpers, already instantiating the values and doing the dirty work for the consumer. However, continuing with this approach is limiting the user to a sub-optimized API. So I bring the idea of from this new API we bring 1:1 wrappers and through them we create new helpers (with sugar and without performance guarantee). I'll close this PR and soon send another one with this idea on top of your work. In any case, it's a thing that should be addressed in the rest of the codebase, both for new and existing APIs. |
excellent,l saw your pr. |
Add GetNextKey() and GetValueAndDeleteKey() for map and map-low.