-Mark
Phil Steitz wrote:
Currently, if you construct a StorelessUnivariateStatistic using an external moment, calls to increment() (silently) do nothing. If we are going to disallow increment() for these stats, it would probably be better to throw IllegalStateException or some other exception to indicate that the operation is invalid.There is a point here, though it may be a little awkward. When you construct the statistic with an external moment, the purpose is to reuse the moment across a number of statistics, in which case the increment method is only called on the moment, not on the statistics. This is only clear in the documention right now, throwing an IllegalStateException with a really strong message would better alert the developer.
It looks to me; however, like we could actually allow the increment to work in this case, delegating to the moment. What am I missing here?
+1
Is this +1 for throwing or for allowing the increment, delegating to the moment?
That's the real problem -- local variables for methods should not be instance variables and certainly not part of the API (which they are as protected instance variables).
One more small change that I would like to make is to eliminate the protected "helper" stats used in evaluate() (e.g. the Sum instance used by the Mean), making these local to the evaluate methods. This is because a) there is no need for them to be constructed for each instance and b) making them instance variables may give the (false) impression that their state is related to the state of the enclosing statistic.
This is probibly a premature over-optimization in some cases. But, we would probibly save a couple method calls by localizing the summation. I think your right about the confusion that the Sum object contains some state.
Back when I started the Univariates, I was actually pushing to
have the evaluate methods be static. Then they could be used without Instance objects, the trouble was that they didn't end up in the Interface then. One possibility would be to maintain static counterparts in the Implementation of each statistic. This would give more flexibility to the API in that the user could take advantage of static calls where they are beneficial (instances such as this).
Of course, another option would be to move the common code back into StatUtils if these methods are going to be static. That would add efficiency to StatUtils as well.
Phil
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
