BIND 10 #1604: reorganize RRset class hierarchy to allow further optimization

[BIND 10 Development]

#1604: reorganize RRset class hierarchy to allow further optimization
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120207
                  Component:         |            Resolution:
  libdns++                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:  auth   |           Total Hours:  0
  performance                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => vorner


Comment:

 > First, I'm not sure it is the best thing to do to throw !NotImplemented
 on getRRsig
 This is only in the BasicRRset class.  The idea here is that as BasicRRset
 knows absolutely nothing about signatures (these are introduced in the
 RRset subclass), if you're referencing one and using a BasicRRset you've
 made a mistake.

 For simplicity I added the signature methods to AbstractRRset.  Another
 way of doing it would be to define a separate SignedRRset class.  Jinmei
 suggested a
 [https://lists.isc.org/pipermail/bind10-dev/2012-January/003004.html
 SignedRRset class under AbstractRRset].  A purer separation of the
 signatures from the RRset would be to make SignedRRset a separate
 (abstract) class and have RRset inherit from both BasicRRset and
 SignedRRset.  However, given that we don't even use BasicRRset on its own,
 I doubt if the added complexity is worth it.

 (Your [https://lists.isc.org/pipermail/bind10-dev/2012-January/003005.html
 comment on the mailing list] about the lack of need for a BasicRRset is
 valid: we don't use it, it would be simplest to merge it with RRset and
 have have a two-level hierarchy.  However, I elected to make as few
 changes as possible when doing this ticket - we can always create another
 ticket for that.)

 > With the addRRsig methods, would it be better to give the parameter
 (like {{{rdata::ConstRdataPtr}}}) by reference?
 This is a point of discussion between myself and Jinmei - see the comments
 in #1305.  It has not been fully resolved but currently no change is being
 made.  (Having said that, where a {{{ConstXxxxPtr}}} is not being
 modified, passing the argument as {{{const ConstXxxxPtr&}}} might overcome
 Jinmei's objections.)

 > I don't know if there's much we can do about it, but the part with
 Associated RRsig methods in BasicRRset looks too much like a copy-paste,
 including the documentation. I fear that if we ever need to change
 anything there, we'll forget to change all the copies and we'll get
 inconsistency.
 True, but at the moment if we don't copy the information, Doxygen will
 complain.

 One other thing: suggested text for the !ChangeLog is
 {{{
 Alter RRsetPtr and ConstRRsetPtr to point to AbstractRRset (instead of
 RRset)
 to allow for specialised implementations of RRsets in different data
 sources.
 }}}

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