[Simplex tree] Const Simplex_tree #1119
Conversation
There was a problem hiding this comment.
Ah, I thought you wanted to make it more similar to standard containers that have both iterator and const_iterator, but here you are basically moving from nothing is const to everything is const (Dictionary_it is not public). It doesn't look bad, but I'll need to think a bit.
src/Persistent_cohomology/include/gudhi/Persistent_cohomology.h
Outdated
Show resolved
Hide resolved
src/Simplex_tree/include/gudhi/Simplex_tree/Simplex_tree_star_simplex_iterators.h
Show resolved
Hide resolved
| @@ -1489,7 +1590,7 @@ class Simplex_tree { | |||
| { | |||
| Node& node_u = static_cast<Node&>(node_as_hook); //corresponding node, has label u | |||
| Simplex_handle sh_u = simplex_handle_from_node(node_u); | |||
| Siblings * sib_u = self_siblings(sh_u); | |||
| Siblings* sib_u = const_cast<Siblings*>(self_siblings(sh_u)); | |||
There was a problem hiding this comment.
Needing const_cast here looks ugly. Maybe self_siblings should still return a non-const pointer, at least if the Simplex_tree is not const?
There was a problem hiding this comment.
As the simplex handle points to a constant Node, we need somehow to "unconst" it. But I could hide the const_cast in self_siblings. Or make simplex_handle_from_node return a non const iterator instead of a simplex handle.
…l into const_simplex_tree
Possibility to have a
const Simplex_tree, but still be able to iterate over it or access information.It is a draft, as it probably needs more testing (ignore the tests added to
simplex_tree_unit_test.cpp). But I would like to have an opinion on it and see if I didn't forgot something obvious.The modifications should not impact old code, if I didn't missed something important. The only big difference is that
Simplex_handleis now aconst_iteratorinstead of aniterator. If the users used the interface as expected, than there should be no problems. It will only be a problem if a user used theSimplex_handleas an iterator instead of an handle, ie, directly didsh->second.assign_filtration(1)instead of usingst.assign_filtration(sh, 1). Which is a bit risky and therefore unlikely?