BIND 10 #510: Implement dictionary-type variable size counter
BIND 10 Development
do-not-reply at isc.org
Mon Dec 5 11:38:17 UTC 2011
#510: Implement dictionary-type variable size counter
-------------------------------------+-------------------------------------
Reporter: | Owner: y-aharen
stephen | Status: accepted
Type: | Milestone:
enhancement | Sprint-20111206
Priority: major | Resolution:
Component: | Sensitive: 0
statistics | Sub-Project: DNS
Keywords: | Estimated Difficulty: 3.0
Defect Severity: N/A | Total Hours: 0
Feature Depending on Ticket: Auth |
statistics |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by y-aharen):
First, I apologize for late response. And thank you for the reviewing.
Replying to [comment:6 jelte]:
> I have taken the liberty to commit and push a few indentation fixes, and
I
> changed the logger initialization call in run_unittests.cc, see commit
657349ae281dcdf737b187d0be2cd7d0e4fa92a7
Thank you. The changes looks OK for me too.
> In general, the code looks OK. I do wonder why we need an implementation
of a
> dictionary, wasn't a direct std::map enough? (or is it impossible to use
the
> elements by reference then?)
In fact, I haven't implemented a dictionary, just using
boost::unordered_map.
I'd like to define a concrete interface. A direct std::map exposes much
information about internal implementation, so I decided to define an
interface to manipulate a set of counters.
> counter_dict_unittests.cc: if the test fixture doesn't do any setup or
cleanup,
> and holds no state, it's really not nessecary, and the tests can also be
done
> with the TEST() macro instead of the TEST_F() macro. In this case,
nearly every
> test however does the same initial thing (define
counters(NUMBER_OF_ITEMS)), so
> that could actually go into the fixture.
Yes, the tests requires initialization, the fixture is needed. If you have
any suggestion, please let me know.
> counter_unittests.cc: perhaps the incrementCounterItem test can also try
another
> round of inc() calls after the three EXPECT_EQ()'s, then check them
again, so
> that the tests also make sure they do not return fixed values.
I've modified the test as you've suggested. Thank you.
I also made a trivial fix to change the way to define enums, as figured in
coding guidelines.
If the branch is OK, I would like to merge it into master, to begin the
following tasks in the next sprint.
--
Ticket URL: <http://bind10.isc.org/ticket/510#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list