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