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