commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark R. Diggory" <mdigg...@latte.harvard.edu>
Subject Re: [math] Moving apply() down DescriptiveStats hierarchy, simplifying structure
Date Sun, 18 Jan 2004 19:22:06 GMT
Here's a patch that does the list of changes I stated below. Phil, now 
that is see it, I'm not so sure I'm as against it as I was before. It 
does seem to make the implementations a little bit more clearly defined. 
And I think your right about if users want a window they can use 
DescriptiveStatisticsImpl.

-Mark

Mark R. Diggory wrote:

> 
> 
> Phil Steitz wrote:
> 
>> "Mark R. Diggory" <mdiggory@latte.harvard.edu> wrote:
>>
>>> I think your percieving apply(...) as if its to "apply" externally 
>>> instantiated UnivariateStatistics, it wasn't designed for that, it 
>>> was designed to polymorphically implement the evaluations for the 
>>> individual internal existing statistics which are incremented in 
>>> addValue(...).
>>
>>
>>
>> The problem is that the way the API is written, it takes an arbitrary
>> Univariate as argument.  It makes sense to apply() any Univariate when
>> there are stored values to apply it to, or if the actual parameter is one
>> of the internally maintained stats (in which case you can use the getXxx
>> methods instead); but it makes no sense when there is no storage and 
>> <stat>
>> is not one of the internally maintained instances (or at least of the 
>> same
>> type as one of these or somehow computable from the internally maintained
>> state).  If we keep it, we should throw or return NaN if there is no 
>> window
>> and <stat> is not one of the internally maintained stats. 
> 
> 
> I agree, thats why it is not part of the DescriptiveStatistics interface 
> because it was never meant to be used by the user.  This is why I 
> recomend making it protected, its part of the Abstract classes so that 
> its available to the implementations internally. Its the responsability 
> of the developer to make sure its applied properly, the user will not be 
> making any mistakes by trying to use it because its not available to 
> them in the DescriptiveStatistics interface.
> 
>> My preference would be to move this down to the Stored version where it
>> always makes sense. I understand that the loss here would be in applying
>> additional Univariates to the finite window case, but the same
>> functionality is available in the stored impl.
> 
> 
> I'm not thrilled about it, but continue to entertain the idea:
> 
> If we do this we will have to do the following:
> 
> 1.) We will end up having to reimplement every 
> DescriptiveStatistics.get<Stat> method in 
> AbstractStorelessDescriptiveStatistics to call 
> StorelessUnivariateStatistic.getResult() if we remove the abstract 
> apply() method from the underlying AbstStorlessDescrStat implementation.
> 
> 2.) We will then need to reimplement every get<Stat> method in 
> AbstDscrStats to overload the above get<Stat> method in 
> AbstStrlsDscrStats to call "apply" again.
> 
> 3.) We will need to overload get/SetWindowSize in StorlsDscrStatImpl to 
> throw UnsupportedOperationException's
> 
> 4.) We will remove the windowed storage from StorlsDscrStatImpl.
> 
> In the end, this reduces the functionality of StrlsDscrStatImpl and 
> makes it have even more "usupported operations" than it did before.
> 
> I'd like to hear what other math developers / contributors think of this.
> 
> -Mark
> 

-- 
Mark Diggory
Software Developer
Harvard MIT Data Center
http://osprey.hmdc.harvard.edu

Mime
View raw message