BIND 10 #2750: support DomainTree::remove()

[BIND 10 Development]

#2750: support DomainTree::remove()
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:  muks
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  data source   |                    Milestone:
            Keywords:                |  Sprint-20130917
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DNS           |                 CVSS Scoring:
Estimated Difficulty:  7             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => muks


Comment:

 Hello

 Replying to [comment:17 muks]:
 > > The name of methods and comment suggests it would set the pointers
 > > mutually (child to parent and parent to child), but it is not
 > > so. Should the comment be reformulated and/or renamed?
 > > {{{#!c++
 > >     /// \brief Helper method used in many places in code to set
 > >     /// parent-child links.
 > >     void setParentChild(DomainTreeNode<T>* oldnode,
 > > }}}
 >
 > Re-reading the code, all I can come up with is `setParentsChild()` (with
 > an extra `s`) which is not much of an improvement. Can you suggest a
 > name?

 That `s` would indeed be an improvement. Or something like connectChild?

 > > This seems to be unconditional. Is that OK?
 > > {{{#!c++
 > >         other->setParentChild(this, other, root_ptr);
 > > }}}
 >
 > I didn't understand this. What is wrong here?

 I'm not sure something is wrong. But this sets the parent's child without
 any condition. Is it possible the lower might not be direct child of
 root_ptr in the modified tree? If not, why?

 > > The same kind of double indirection is with the `find` methods.
 >
 > I'm not sure if we can avoid the `find()` changes as they are `public`
 > methods. Or am I missing the point?

 The point is exactly the same as abstractAccessorImpl. I don't think we
 need the `findImpl` wrappers, these can be skipped and the resulting
 `tree->find` called directly. These are private.

 > > I think this was copy-pasted so many times someone (not necessarily
 > > you) lost the explicit root mentioned in the comment.
 > > {{{#!c++
 > > // The full absolute names of the nodes in the tree with the addition
 of
 > > // the explicit root node.
 > > const char* const domain_names[] = {
 > >     "c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h",
 "o.w.y.d.e.f",
 > >     "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f", "k.g.h"
 > > };
 > > }}}
 >
 > From how I read it, the comment seems to say that these are the names in
 > the tree, along with the "." node. If the "." node was already there, it
 > wouldn't say that explicitly.

 But then, would it say /explicit/ root there? The root node is implicit in
 this list.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2750#comment:19>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development