On Tue, 3 Jan 2012 17:52:34 +0100, Kinkie wrote:
On Tue, Jan 3, 2012 at 3:32 AM, Amos Jeffries <[email protected]>
wrote:
On 3/01/2012 10:11 a.m., Kinkie wrote:
Hi all,
here's a new interim merge proposal for the StatHist refactoring
effort. This merge aims to close bug 3381 and advance a bit the
general StatHist refactoring effort.
What it does:
- change the type used for StatHist counters to uint64_t
(parametric
as StatHist::bins_type)
- change the index for StatHist bins to unsigned
- bring stub_StatHist forward and actually use it, also to remove
some
objects from tests
What crept in:
- an initial effort for a StatHist unit test.
- some src/Makefile.am cleanup
in StatHist.cc:
* StatHist::findBin() - float type is quite inaccurate. Can you use
double?
Yes. Thanks
* StatHist::operator =() - rather than duplicating this code can
you make
it inline?
I'm not sure I understand what you mean.
You have duplicate code in StatHist.cc and stub_StatHist.cc with notes
about keeping them in sync. that is prime candidate for inlining that
particular duplicated function. The whole point of having stub_* is to
prevent an active version of the stubbed object being linked. If the
stub code gets run the test is broken.
So this method function should be inline if it needs defining when the
stub object definitions are linked.
Please keep in mind that this code is currently in due to the need to
satisfy the squid3 mandatory coding guidelines (StatHist has default
constructor and destructor, so it must have assignment).
I know. That is irrelevant to this particular code duplication issue.
in testStatHist:
* please define testStatHistEnum() or replace with a TODO.
At the cost of a bit of further creep-in, done. Here's a revised
patch, which on top of the previous one, also:
- actually uses the unit test
- delivers a complete default constructor for StatHist
- adds equality test for StatHist
- adds more stubs to the collection (stub_mem and stub_stmem) and
uses them
Will review later.
Amos
