commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Meyer" <meyer...@gmail.com>
Subject RE: [Math] MATH-894
Date Mon, 12 Nov 2012 19:38:07 GMT
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.steitz@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message