BIND 10 #2206: define and implement ZoneTableSegment class

[BIND 10 Development]

#2206: define and implement ZoneTableSegment class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  UnAssigned
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121009
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''zone_table_segment.h'''

 - brief descriptions are mostly meaningless:
 {{{#!cpp
 /// \brief Zone Table Header Class
 struct ZoneTableHeader {
 }}}
   It's too obvious that this is a "Zone Table Header" class (or
   struct) from the type name.  I'd give some more informative one-line
   summary of what this class is.  I'd also provide some longer version
   of description to this particular class.  Some points also apply to
   other classes.
 - the return value description is mostly redundant:
 {{{#!cpp
     /// Returns the MemorySegment used in this zone table segment.
     ///
     /// \return Returns a ZoneTableHeader for this zone table segment.
 }}}
   I'd either provide more meaningful description or if there really
   isn't any just skipping the detailed version of the description.
 - `ZoneTableHeader`: maybe it's better to provide an accessor to
   table rather than having the caller be aware of offset_ptr (which is
   a kind of details that the caller doesn't have to care about):
 {{{#!cpp
 struct ZoneTableHeader {
 public:
     ZoneTable* getTable() { return (table.get()); }
     // maybe we need a const version too?
 private:
     boost::interprocess::offset_ptr<ZoneTable> table;
 };
 }}}
 - getHeader() should never return NULL.  I'd note that (or maybe we
   should return a reference instead of a pointer?  I'm not sure which
   one is better).
 - I'd note that the object returned by create() is dynamically
   allocated and the caller is responsible for deleting.  (providing a
   corresponding `destroy()` method maybe cleaner).
 - maybe a matter of taste in practice, I'd explicitly define the
   constructor of the base class as private to make it clear that the
   factory must be used to instantiate it.
 - not fully sure about this right now, but we probably want to have a
   const version of `getHeader()`.  The lookup code path should only
   need read-only access to the table.

 '''zone_table_segment_local'''

 - getHeader() and getMemorySegment(): for such very simple methods we
   might just define them in the header file.  It's purely a taste
   matter though, as they are virtual methods.

 '''zone_table_segment_unittest'''

 - (general) scoped_ptr would be a slightly better choice than auto_ptr
   for such usage.
 - `config` is generated in every test.  You can avoid by introducing a
   fixture.  The total amount of code length wouldn't be much different
   though.

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