-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
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).
/* in Sum */ public double evaluate( final double[] values, final int begin, final int length) { return Sum.eval(values,begin,length); }
public static double eval(
final double[] values,
final int begin,
final int length) {
double sum = Double.NaN;
if (test(values, begin, length)) {
sum = 0.0;
for (int i = begin; i < begin + length; i++) {
sum += values[i];
}
}
return sum;
}/* in Mean */
public double evaluate(
final double[] values,
final int begin,
final int length) {
if (test(values, begin, length)) {
return Sum.eval(values, begin, length) / ((double) length);
}
return Double.NaN;
}This would eliminate the instance var while still encapsulating the summary code for reuse.
-Mark
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
