On 16/12/2011 7:05 p.m., Kinkie wrote:
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?
None from me now.
Amos
