Please keep ResizeableDoubleArray as its own class. I find it very useful for more than descriptive statistics. I do like the idea of adding an apply method to it.
Patrick -----Original Message----- From: Phil Steitz [mailto:phil.ste...@gmail.com] Sent: Monday, November 12, 2012 1:23 PM To: Commons Developers List Subject: Re: [Math] MATH-894 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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
