Open vinipsmaker2 opened 6 years ago
I forgot to fill more details. it
is the value returned from obj.key_begin()
and obj
is dynamic::variable
Good catch.
The resolution gives me some headache though. There are two solutions we can use:
key_iterator erase(key_iterator)
member function.key_iterator
to const_iterator
, e.g. key_iterator::base() -> const_iterator
.The first solution enables the user to loop over all elements and erase some of them. That is:
for (auto it = v.key_iterator(); it != v.key_end(); /* increment done below */)
{
if (should_we_erase_element(*it))
it = v.erase(it); // erase returns a key_iterator
else
++it;
}
The second solution has the advantage of being more general, so it can be used for other functions, like insert()
. However, the example above cannot be used.
I am leaning towards the second solution because:
iterator erase(const_iterator)
which was introduced in C++11 (via proposal N2350.)reverse_iterator::base()
.Why can't the example compile with solution #2
?
Because it
has type key_iterator
and erase
returns type iterator
.
There is no conversion from iterator
to key_iterator
because the latter needs an extra member variable.
Regarding your comment about parsing the entire input to a dynamic::variable
, you will be pleased to know that I am working on a json::parse
and json::format
that works on dynamic::variable
without the need for Boost.Serialization.
We already have the looping problem with iterator erase(const_iterator) which was introduced in C++11 (via proposal N2350.)
Okay. I've read N2350 (except for the proposed wording chapter).
I think if we apply the same solution here, the proper solution would be to add key_erase
to match the begin
/key_begin
split. key_erase
would receive a const_key_iterator
and return a key_iterator
.
I wouldn't mind to have a overloaded erase
function instead typing the key_
thou.
Regarding your comment about parsing the entire input to a
dynamic::variable
, you will be pleased to know that I am working on ajson::parse
andjson::format
that works ondynamic::variable
without the need for Boost.Serialization.
Surely I'll enjoy that.
I have added a key_iterator::base()
function to convert a key_iterator into a const_iterator in db775c48ad3624b7570f9bbd4512bed6ae238c27, so now v.erase(kit.base())
is doable.
Regarding the key_erase()
function, I would rather have such functionality as algorithms, e.g. like the dynamic::key::count()
and dynamic::key::find()
algorithms, to prevent a proliferation of the variable
interface.
Okay. Let's leave the issue open for some time in case others want to pop in and add their comments.
I have added a
key_iterator::base()
function to convert a key_iterator into a const_iterator in db775c4, so nowv.erase(kit.base())
is doable.
This will suit me for now. Probably I'll send a PR in the following weeks with one dynamic::key::erase
. The question of a sweet spot here between convenience and a clean interface is a tricky one. Not many ready guidelines we can adhere to. But also, it's the problems we always face with novel design (otherwise they wouldn't be novel).
Tested on commit cea147e0523a13c39a9d4b10a76e7d927a2397d6, but looking at the source code from latest develop, I think it should be failing too. I can write an isolated case to test it on
develop
later.Compiler is clang. Error is:
Also, finally I am coding the part where performance is not the critical and I can do whole parses and get the convenience of
dynamic::variable
on my code 🎉🎉🎉.