BIND 10 #1748: define AbstractRRset::isSameKind() and implement the default version

BIND 10 Development do-not-reply at isc.org
Tue Mar 13 03:58:23 UTC 2012


#1748: define AbstractRRset::isSameKind() and implement the default version
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  muks
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  high   |  Sprint-20120320
                  Component:         |            Resolution:
  libdns++                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:  auth   |           Total Hours:  0
  performance                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by muks):

 Replying to [comment:9 stephen]:
 > However, since the return value is boolean, and since each comparison
 generates a boolean value that can be combined with the other values via
 logical operators, I suggest that the following is clearer:
 > {{{
 > bool
 > AbstractRRset::isSameKind(const AbstractRRset& other) const {
 >     return (getName() == other.getName() &&
 >             getType() == other.getType() &&
 >             getClass() == other.getClass());
 > }
 > }}}

 Changed the code to look like this now.

 > isSameKind() can be declared const as it doesn't change anything in the
 class, and the methods it calls are const.

 Done :)

 >
 > '''src/lib/dns/tests/rrset_unittest.cc'''[[BR]]
 > I would add one check: that an RRset is the same kind as itself, i.e.
 add
 > {{{
 > EXPECT_TRUE(rrset_w.isSameKind(rrset_w));
 > }}}

 Done :)

 > '''src/lib/datasrc/rbnode_rrset.h'''[[BR]]
 > RBNodeRRset::isSameKind():
 > * "rb" can be declared and initialized in the same line.
 > * Is there a need for "t" - can't "&other" be put as the argument to
 dynamic_cast?
 > * As "rb" is not "bool", I would prefer the check "rb != NULL".
 > * Even single-line "if" statements should include braces.
 > * Value returned with "return" should be enclosed in brackets.

 Done :)

 > The code is an optimisation for comparing RBNodeRRsets.  However, in
 doing this optimisation, semantically the code is not "is same kind" but
 is instead "is identical object".  I think it deserves a comment
 explaining why there is this difference.

 Added :)

 > '''src/lib/datasrc/tests/rbnode_rrset_unittest.cc'''[[BR]]
 > isSameKind test - can remove the test on rrset_w (it just duplicates the
 test done on rrset_x).

 I've changed the test to check a different name now.

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


More information about the bind10-tickets mailing list