BIND 10 #2435: implement datasrc version of RRsetCollection

[BIND 10 Development]

#2435: implement datasrc version of RRsetCollection
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130205
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:25 muks]:

 I made a few minor editorial cleanups and committed proposed update to
 pydoc text.  The latter is basically just pretty-formatted C++ doc
 with trivial adjustments (only when necessary, and they are
 commented).  So these should be okay, but a double check would be
 nice.

 With these changes please go merge (I guess pydoc updates will cause
 conflicts; but you can just replace the whole text with the version of
 this branch).

 > > Then `ZoneUpdater` won't depend on `datasrc::RRsetCollectionBase`.
 > >
 > > > > - destructor doesn't have to be defined explicitly in this case.
 > > >
 > > > There was a discussion on Jabber about 2 weeks back on whether to
 > > > include empty destructors anyway in derived classes to show up the
 > > > `virtual` keyword (though it's unnecessary). I don't know what the
 > > > coding style is here, so based on that discussion I added it.
 > >
 > > I wasn't in that discussion at the exact time and missed some part of
 > > it, but according to the very last part of it it's more about this:
 > > http://bind10.isc.org/wiki/CodingGuidelines#VirtualMethods
 > > rather than whether to explicitly define empty derived destructors.
 >
 > I read the coding guidelines before making that change, and if you have
 > all of the discussion you will see I mentioned to Jelte that the empty
 > destructor declaration (with or without `virtual`) was unnecessary in
 > the derived classes. The coding guidelines aren't always complete or
 > have something I agree with, so I just follow whatever is specified for
 > consistency. From the discussion, I thought it was expected that we add
 > the virtual destructor to every derived class.

 Hmm, some points are not clear to me: (as you probably know) an empty
 destructor normally doesn't require an explicit declaration or
 definition: the (probably) only exception is the very base class that
 is expected to be inherited.  And, for more trivial classes omitting
 it can be even desirable for performance reasons.  For a derived class
 neither of the points applies, so that would be more a matter of
 taste.  I don't know what kind of arguments made you think we expect
 to define derived class destructors explicitly (not seeing the whole
 part of that discussion), and although I'd rather omit them for
 conciseness, but that's not a strong preference.

 Anyway, the latest version of the branch doesn't have them, which is
 fine to me of course, and even if they appear on merge, I don't
 object.

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