commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [Math] MATH-894
Date Mon, 12 Nov 2012 19:47:41 GMT
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.

> Patrick
> -----Original Message-----
> From: Phil Steitz [] 
> 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:
>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message