Overall I think the old math-statistics functioned well and I would not  be
inclined to mess with the old object hierarchy without reason. But there
are some strange design choices in this code. Mean() is used here as an
example.

1) In Mean() the two constructors create a FirstMoment() object:

--
/** Constructs a Mean. */
    public Mean() {
        incMoment = true;
        moment = new FirstMoment();
    }

    /**
     * Constructs a Mean with an External Moment.
     *
     * @param m1 the moment
     */
    public Mean(final FirstMoment m1) {
        this.moment = m1;
        incMoment = false;
    }
--

And then this moment object, as far as I can tell, is never used in the
mean calculation at all:

---

@Override
    public double evaluate(final double[] values, final int begin, final
int length)
        throws MathIllegalArgumentException {

        if (MathArrays.verifyValues(values, begin, length)) {
            Sum sum = new Sum();
            double sampleSize = length;

            // Compute initial estimate using definitional formula
            double xbar = sum.evaluate(values, begin, length) / sampleSize;

            // Compute correction factor in second pass
            double correction = 0;
            for (int i = begin; i < begin + length; i++) {
                correction += values[i] - xbar;
            }
            return xbar + (correction/sampleSize);
        }
        return Double.NaN;
    }
---

FirstMoment just seems  to be in the constructor for its own sake.

I think  it would make more sense to at the very least make Mean and
similar classes very lean, and see if there are any use cases for which
this is not sufficient.

2)

Mean extends AbstractStorelessUnivariateStatistics. I have no  problem with
the class being Storeless, which I think means immutable. But, in that case
the methods might as well be static.  However the old grammar of

new Mean().evaluate()

is only marginally different from what the static grammar would be:

Mean.evaluate

-Eric

Reply via email to