Boost logo

Boost :

From: Marcin Kalicinski (kalita_at_[hidden])
Date: 2006-04-21 19:59:22

Hi Matias,

Thank you for a great review.

You may have noticed I have already replied to similar proposal of Jeff
Flinn, somewhere in review thread. Now, as you presented the implementation
and interface in so much detail, I must definitely reconsider. Previously I
only thought of a path class as means of encapsulation for default
separator. And your proposition goes much further. The idea with overloading
operator / looks very nice to me. It will remove the need for default
separator altogether (read: halve the number of overloads). It will also
make the lookup faster.

I'm thinking how to merge it with another important suggestion, which is to
allow paths of arbitrary type, not only strings (for example arrays of ints,
where each int is an index on its level, in some cases this might be
useful). Having the path class, whole path parsing should be done by path
objects. Probably the best approach would be to templatize basic_ptree on
path_type and require certain functionality from it. This is already done,
except that instead of path we have a string functionality that is needed.

> If we have to make a change that compromise the interface i think we
> are going to kill it.

I think you are right.

> Only one thing, i dont like to pass a bool as a function parameter
> because i always forget what was intend for. You use this aproach to
> specify if the path must be overwritted. I thinked about it and not
> reach to any other good sintaxis, but it is there.

There is a different approach, vastly superior to the extra bool parameter.
Just add "set" functions in addition to "get" and "put". "set" will always
replace existing property if it is there, while "put" will always add new
one. The only reason why I didn't implement it is because the get/put
interface was already bloated, and adding "set" looked like a step in wrong
direction. Now, that I think we can get rid of those ugly separator
overloads (sigh), this may be an option again.

Additionally the library should have a get/put-style member function to
delete an existing property. It obviously has erase, but it does not work
with paths. We would end up with get/set/put/del(?), and the picture would
be, hopefully, complete.

Again thank you for a great review and best regards,

Boost list run by bdawes at, gregod at, cpdaniel at, john at