On 11/12/12 11:38 AM, Patrick Meyer wrote: > 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.
Thanks for speaking up :) I think Gilles is right though on the broken encapsulation, so maybe its best to add the apply or "applyStatistic" method as described below. Phil > > 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 > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
