BIND 10 #461: Empty node processing in MemoryZone Easy Part

[BIND 10 Development]

#461: Empty node processing in MemoryZone Easy Part
-------------------------------------+-------------------------------------
                 Reporter:  hanfeng  |                Owner:  hanfeng
                     Type:           |               Status:  reviewing
  enhancement                        |            Milestone:  A-Team-
                 Priority:           |  Sprint-20110126
  critical                           |           Resolution:
                Component:  data     |            Sensitive:  0
  source                             |  Add Hours to Ticket:  0
                 Keywords:           |          Total Hours:  0
Estimated Number of Hours:  0.0      |
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => hanfeng


Comment:

 Hello

 I have some comments:

 It is not OK to place using namespace into a header file, since everyone
 who includes it is forced to it as well, even if he does not want.

 This comment looks really strange:
 {{{
 /// \par Depending on different usage, rbtree will support different
 /// search policy current implementation support whether empty
 /// node is visible to end user or not This is as the last template
 /// parameter for end user, choose ReturnEmptyNodePolicy to make
 /// return non-empty, choose ReturnEmptyNodePolicy to get empty node
 }}}

 It seems to miss punctuation and some parts seem to be there twice.
 Therefore I have quite a lot trouble guessing what it should mean.

 This is no longer true:
 {{{
 /// \note open problems:
 /// - current find funciton only return non-empty nodes, so there
 ///   is no difference
 ///   between find one not exist name with empty non-terminal nodes,
 ///    but in DNS query
 }}}

 Also, you reformatted most of the documentation comments. You will
 probably have some trouble merging them with the comments currently in
 master, since they have been quite rewritten. Could you, please, merge
 master into your branch and resolve it? Also, you changed behaviour of
 insert, so the comment there should be updated to reflect that.

 The whole concept of another class template argument is somehow
 overcomplicated. It is true that any recent compiller will inline it
 completely, therefore eliminate the:
 {{{
 if (SearchPolicy::returnEmptyNode() ||
 }}}
 condition, but that is hard to follow (even more as the class is forward-
 declared without any reason). I'd suggest either template specialization
 or having the template argument be bool, not class:
 {{{
 template<typename T, bool ReturnEmptyNode = false>
 }}}

 Is the #include <stack> used anywhere?

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