breese / trial.protocol

Network wire protocols
11 stars 4 forks source link

Add key::erase algorithm #28

Closed vinipsmaker closed 6 years ago

vinipsmaker commented 6 years ago

Could you give me a hint on how to create a dynamic::key_iterator from a map::iterator? I understood the current union containing the iterator, but I could make use of a suggestion on where to add such conversion (add a new constructor to key_iterator, make the new algorithm a friend function...).

breese commented 6 years ago

It is easier to implement key::erase as follows:

template <template <typename> class Allocator, typename T>
auto erase(basic_variable<Allocator>& self,
           const T& other) -> typename basic_variable<Allocator>::key_iterator
{
    switch (self.symbol())
    {
    case symbol::map:
        {
            auto where = key::find(self, other);
            auto result = where;
            ++result;
            self.erase(where.base());
            return result;
        }

    default:
        return self.key_end();
    }
}
vinipsmaker commented 6 years ago

I've modified the PR to finish the tests and implement like you suggested. The PR is ready for review.

Also, I'll probably do more PRs related to this one.

One more thing: given you're the author of the function, I've committed using your name (git makes this task easy).

breese commented 6 years ago

Regarding string_view do notice that we have include/trial/protocol/core/detail/string_view.hpp to select the right Boost version.

breese commented 6 years ago

Regarding key::find there is a private constructor in the iterator_base class that takes a map::iterator, but I want to avoid exposing a map::iterator to key_iterator conversion in the API.

vinipsmaker commented 6 years ago

Regarding key::find there is a private constructor in the iterator_base class that takes a map::iterator

Nice.

but I want to avoid exposing a map::iterator to key_iterator conversion in the API.

Agreed.

What about a friend function living in detail namespace? It'd make it easy to access it across our free functions that are intended to be used with dynamic::variable.