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

 Replying to [comment:9 jinmei]:
 > I'm afraid I don't understand the question...what do you mean by "when
 > there is code in the structure in our coding style"?  Regarding the
 > use of `struct` (instead of `class`) itself, I don't think there's any
 > written guideline for BIND 10.  So far, the choice is up to the
 > developer.  Personally, I tend to use `struct` when I use it as a mere
 > composition of members, not providing any behavior through non trivial
 > methods (and the case of `ZoneSpec` should meet this personal
 > convention).  It's probably better if we have some unified policy on
 > in which case we use `struct` or `class` as it might possibly cause
 > some confusion, so I'm not opposing to it.  So far, though, I don't
 > have a feeling that this has been a serious issue.

 I wanted to ask about use of `struct` keyword instead of `class`, when
 there are member methods like constructors. This explanation is fine; I
 just wanted to know if we followed any preference for it.

 > > * 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.
 >
 > On a closer look, I realized `it_begin_` is clearly unnecessary, so I
 > removed it.  As for `ZoneTableAccessor::end()`, I suspect it's not
 > easy, if not impossible, to cleanly implement it for database-based
 > data sources.  As documented, this was the main reason why I didn't
 > make the whole interface std::iterator compatible.  And, without
 > `end()`, we'll need something like `it_end_` for the cache
 > implementation or some other overhead either in speed or space anyway,
 > so I think it makes to keep the current approach.  I also think the
 > space overhead for `it_end_` isn't a big issue, because standard
 > iterators should generally be small enough, we use
 > `ZoneTableAccessor::Iterator` in the form of a pointer and won't use
 > too many of them at the same time.

 This sounds reasonable to me.

 > > * I pushed some minor comment update commits to the branch. Please
 check
 > >   them.
 >
 > These are generally good or okay (for some I wouldn't change them that
 > way, but I don't object).

 I push comment patches directly because it's faster to convey to you the
 many little changes as patches than discussing them individually in the
 ticket. But if you think something is unnecessary or feels incorrect,
 please revert it.

 I think you can go ahead and merge this branch if you don't intend to make
 any changes.

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