BIND 10 #2432: define and implement base and libdns++ version of RRsetCollection

[BIND 10 Development]

#2432: define and implement base and libdns++ version of RRsetCollection
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  medium        |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130108
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi Jinmei

 Replying to [comment:15 jinmei]:
 > Please cherry-pick 823f41b on the trac2433 branch.  The original
 > version doesn't compile for my environment (see commit log - this type
 > of portability issues sometimes happen).

 Done.

 Replying to [comment:16 jinmei]:
 > While working on #2433 I've noticed a few things (incorrectly
 > updated #2433 with those comments, now copying them to the correct
 > ticket...)
 >
 > - please add rrset_collection(,_base).h to libdns___include_HEADERS in
 Makefile.am.

 Done.

 > - in RRsetCollectionBase, protected things should generally be
 documented sufficiently detailed so that derived class implementor can
 implement their version from the documentation.

 Done.

 > - Also for iterator. And, btw, per naming convention it should be named
 something like Iterator.

 Documentation added. For renaming to `Iterator`, it seems that for
 collections with `std::iterator`s, the preferred naming is
 `CollectionType::iterator` which is why it was named such. If you still
 think this should be changed to `Iterator`, please tell me and I'll update
 it.

 > - I'd like to suggest defining another version of constructor that takes
 istream. That would be useful for tests (and probably for some other
 general cases).

 Done.

 > - I'd explain what kind of collection is constructed by the default
 constructor.

 I did not understand this.

 > - we need to define a virtual destructor in the base class.

 Done.

 Replying to [comment:18 jinmei]:
 > And yet another...I'd explain the ownership of the passed rrset after
 > the call:
 > {{{#!cpp
 >     /// \brief Add an RRset to the collection.
 >     ///
 >     /// Does not do any validation whether \c rrset belongs to a
 >     /// particular zone or not. It throws an \c isc::InvalidParameter
 >     /// exception if an rrset with the same class, type and name already
 >     /// exists.
 >     void addRRset(isc::dns::RRsetPtr rrset);
 > }}}

 Done. :)

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