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        |
-------------------------------------+-------------------------------------

Comment (by hanfeng):

 Replying to [comment:5 vorner]:
 > 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.

 I have remove branch trac461 and use latest master to create trac461 again
 and modify the code base on it. Since the code has modified a lot
 comparing with origin trac461 which is created quite long time ago.


 > 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>
 > }}}
 >
 Using template parameter is the common way to do policy based programming,
 although current policy is quite simple, but the extensibility is better.
 Also it's quite strange use bool as template parameter, if only one bool
 variable is needed, I prefer to use it as construction function parameter.

 > Is the #include <stack> used anywhere?
 The hard part for this task needs stack, I don't remove it cleanly, now it
 is removed

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