BIND 10 #510: Implement dictionary-type variable size counter

[BIND 10 Development]

#510: Implement dictionary-type variable size counter
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  y-aharen
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20111220
                   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      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => y-aharen


Comment:

 Small note; after addressing review comments, you can assign the ticket
 back to the reviewer directly, there's no need to set it to unassigned
 again (in fact, that might cause it to be missed by the original
 reviewer).


 >
 > > 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.
 >

 Ok

 > > 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.
 >

 I suggest we move the definition of the counter variable to the fixture,
 see my commit
 60fd293717cc45323cfb10cf06d5bd264fa083cc into branch trac510

 > > 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.
 >

 ok, looks good

 >
 > If the branch is OK, I would like to merge it into master, to begin the
 > following tasks in the next sprint.

 Yes, please go ahead

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