Re: [PATCH] StatHist / StatCounters refactoring interim merge

> It still does make sense to check the capacity bounds. Consider A
> initialized to 10 entries, and B initialized to 100 entries copying B into
> A. As I read it now the copy only checks that both are initialized, and will
> allow A at position 11+ to be referenced during the copy. We avoid it so far
> by having fixed histograms in the caller code, but it is not actually a
> guarantee anywhere by the StatHist API.

Agreed.

> What I'm looking for is some surety that the A will either fail or grow to
> meet the B size requirements. Given that the callers are using Hist
> consistenly regarding size I think it can probably be an assert for now
> instead of adding growth complexity.

v2 of the patch grows. It adds 3 lines of code or so, and it was in
the todo list anyways.

>>> * StatHist::findBin()- appears to be able to return -1 array index if
>>> capacity is zero. Please make it return unsigned, and return 0 on the
>>> line
>>> doing (bin = 0) presently.
>>
>> Ok for not returning -1, but may I delay turning signed to unsigned
>> for second round?
>
>
> Fine by me.
>
>
>>> * please review capacity, bins, min, max fields to see which can be made
>>> private and renamed with suffix '_'.
>>
>> I intentionally left them protected as I'd like to implement a class
>> hierarchy.
>> Why the trailing underscore? There is no name conflicts.
>
>
> Fine. Just keep in mind that on the copy constructor when you get around to
> adding "max(...), min(...)" it turns into the libc max/min functions or
> template instantiations.

you convinced me on this one. Renamed.

Any objections to merging?
Thanks

-- 
    /kinkie