BIND 10 #2906: define and implement (generic) ZoneTable class

[BIND 10 Development]

#2906: define and implement (generic) ZoneTable class
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130514
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => jinmei


Comment:

 Hi Jinmei

 Here are my review comments.

 * I got the following compile error when building the branch:
 {{{
 make[6]: Entering directory `/home/muks/bind10/src/lib/datasrc'
   CXX      zone_table_accessor_cache.lo
 In file included from zone_table_accessor_cache.cc:15:0:
 ../../../src/lib/datasrc/zone_table_accessor_cache.h:42:7: error: direct
 base 'boost::noncopyable_::noncopyable' inaccessible in
 'isc::datasrc::internal::ZoneTableAccessorCache' due to ambiguity
 [-Werror]
 cc1plus: all warnings being treated as errors
 }}}

 This could be worked around using the following patch (as
 `ZoneTableAccessor` already derives from `boost::noncopyable`:
 {{{
 -class ZoneTableAccessorCache : boost::noncopyable, public
 ZoneTableAccessor {
 +class ZoneTableAccessorCache : public ZoneTableAccessor {
 }}}

 I went ahead and pushed this, as otherwise it generated an intermixed
 diff with with some comment updates I made in
 `ZoneTableAccessorCache`. :)

 * Just to clarify, we allow the use of `struct` (vs. `class` keyword)
   when there is code in the structure in our coding style, correct? It's
   a nice way to say `ZoneSpec` is fully public, but I want to know if we
   have any (unwritten) restrictions in our coding guidelines about it.

 * In the documentation for `ZoneTableIterator` class, in the following
   text, isn't it the case that they go through all the zones until
   `isLast()` returns true?
 {{{
 /// and that subsequent calls to \c getCurrent() go through all the zones
 /// one by one, until \c isLast() returns false.  The implementation must
 /// support the concept of "empty table"; in that case \c isLast() will
 /// return \c false from the beginning.
 }}}

 Also, it should likely say that "... subsequent calls to \c next() go
 through all the zones..." instead of `getCurrent()`.

 * I am not sure if `it_begin_` and `it_end_` should be members of the
   iterator. They waste space there as they are properties of the
   `ZoneTableAccessorCache` itself. Instead of checking `isLast()`, we
   could test the current iterator for equality against
   `ZoneTableAccessor::end()`. This would lend nicely to any future
   refinement of the iterator too. Then again, maybe it is not
   straightforward to implement this due to other constraints.

 * I pushed some minor comment update commits to the branch. Please check
   them.

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