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

Can dynamic::variable::map_type::erase accept string_view? #20

Open
vinipsmaker2 opened this issue Feb 8, 2018 · 5 comments
Open

Can dynamic::variable::map_type::erase accept string_view? #20

vinipsmaker2 opened this issue Feb 8, 2018 · 5 comments

Comments

@vinipsmaker2
Copy link
Contributor

This is not the same as #19. #19 is a bug. This is a feature request.

The following code fails:

auto a = obj.value<trial::dynamic::variable::map_type>();
a.erase(boost::string_view{});

I have to allocate a std::string to erase the proper element. It'd be nice if it worked with string_view directly.

@breese
Copy link
Owner

breese commented Feb 10, 2018

a is an std:map so you have to ask the C++ standardization body.

@vinipsmaker
Copy link
Contributor

I see. Gonna close the issue then. Once #19 is solved, I'll have a solution for that.

@breese
Copy link
Owner

breese commented Feb 10, 2018

I could add a key::erase_if() algorithm, like the new std::erase_if algorithm.

That could also address the concern in #19.

@breese breese reopened this Feb 10, 2018
@vinipsmaker
Copy link
Contributor

That could also address the concern in #19.

erase would stop once the key is found. erase_if would iterate until the very end. Also, erase on a map could use binary search, which is ruled out for erase_if.

erase_if is an useful algorithm, but it's not close enough to erase(key).

And the limitation of not being able to use string_view can be avoided in more elegant ways. The string_view limitation isn't present in the C++14 std::map::find algorithm which can be used to get an acceptable value to the erase function.

@breese
Copy link
Owner

breese commented Feb 11, 2018

Agreed. It makes sense to add both erase and erase_if.

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

3 participants