On 11/12/12 6:05 AM, Gilles Sadowski wrote:
> Hi.
>
> What do you think about deprecating "getInternalValues"?
> Its only use is in "DescriptiveStatistics". I guess that the current usage
> could save some time (by avoiding the creation of an array and copying the
> elements), so that acces to the internal representation should be retained.
That is why it is there. ResizeableDoubleArray was created to be
the backing store for DescriptiveStatistics. We don't actually use
it anywhere else, so I am wondering if it might in fact be better to
deprecate the entire class and aim to make it a private inner class
of DescriptiveStatistics. That way, the broken encapsulation that
you point out will be less of an issue. The idea behind the class
is to support a "rolling window" of data from a stream that
statistics can be applied to. If we want to keep it separate (or
even not, actually), it might be better for it to expose an apply
method that takes a UnivariateStatistic as an argument and applies
the statistic to the currently defined window. So the following
method from DescriptiveStatistics
public double apply(UnivariateStatistic stat) {
return stat.evaluate(eDA.getInternalValues(), eDA.start(),
eDA.getNumElements());
}
would be implemented in ResizeableDoubleArray (just removing the
eDA. everywhere). Then in DescriptiveStatistics you would have
public double apply(UnivariateStatistic stat) {
return eDA.apply(stat);
}
possibly renaming the version in RDA. But given all of the fuss
over this class that really is just there to serve
DescriptiveStatistics, I think it may be best to just make it a
private inner class of DescriptiveStatistics.
Phil
> If so, I think that the name "getInternalValues" should be changed in order
> to make it explicit that we are exposing an instance's field (whose
> modification will be reflected in the object's state). The suffix
> "...Values" is misleading; I suggest "getInternalArray" since current API
> (and current usage) anyways forbids that another data structure be used.
> [The alternative would be to enhance encapsulation by hiding the internal
> representation altogether, thus removing the methods "getInternalValues()"
> and "start()".]
>
> I also notice that the "clear()" method reallocates the internal array.
> IMO, it is unnecessarily inefficient. If one wanted to get this behaviour,
> one could just create a new object. However, when reusing the same object,
> users could legitimately expect that no allocation occurs and that it is
> only the _contents_ that is discarded.
>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
