BIND 10 #517: Empty node processing in MemoryZone difficult Part

[BIND 10 Development]

#517: Empty node processing in MemoryZone difficult Part
-------------------------------------+-------------------------------------
                 Reporter:  hanfeng  |                Owner:  jinmei
                     Type:           |               Status:  reviewing
  enhancement                        |            Milestone:  A-Team-
                 Priority:  major    |  Sprint-20110126
                Component:  data     |           Resolution:
  source                             |            Sensitive:  0
                 Keywords:           |  Add Hours to Ticket:  0
Estimated Number of Hours:  0.0      |          Total Hours:  0
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by hanfeng):

 Replying to [comment:7 jinmei]:
 > Review on rbtree.h.
 >
 > '''RBTree forward declaration'''
 Removed
 >
 > '''RBNode'''
 >  - why successor() is a public method?  It's not used anywhere, and
 >    the interface is not good from design point of view because
 >    it reveals an internal detail of RBTree implementation (that it's
 >    actually a tree of trees).  The only justifiable reason for this
 >    to be public that I can think of is the convenience for testing,
 >    but the there's no direct test for it.  On a related note, if it's
 Successor should be private which should be only used by rbtree.
 More comment is added

 > '''RBTree class description'''
 >  - this sentence(?) doesn't parse (and s/ture/true/?):
 > {{{
 >  * The default policy is to NOT return an empty node to end user,
 >  * pass ture will get empty node during find is needed.
 > }}}
 Document is fixed
 >
 > '''RBTree::NodeChain'''
 >  - it needs more documentation, including "what it is (in more...
 Implementation is modified, now using fixed size array.

 >
 > '''RBTree constructor'''
 >  - documentation: describe the newly introduce parameter.
 Document is added
 > '''about findEx'''
 >  - First off, why do we need to introduce a new method?  My
 >    understanding is that this will be used only from
 "Ex" means extended version, which is not clear. I have merge it with call
 back version find

 > '''nextNode()'''
 >  - code seems to do what it claims to do, but I'm afraid it's
 >    unnecessarily inefficient: it involves a copy of current node
 NodeChain implementation is modified and no copy now.

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