commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: svn commit: r1086057 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java test/java/org/apache/commons/math/stat/correlation/PearsonsCorrelationTest.java
Date Tue, 29 Mar 2011 15:01:30 GMT
> >>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java
> >>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java?rev=1086057&r1=1086056&r2=1086057&view=diff
> >>> ==============================================================================
> >>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java
(original)
> >>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java
Sun Mar 27 22:06:18 2011
> >>> @@ -225,7 +225,8 @@ public class PearsonsCorrelation {
> >>>       * @throws  IllegalArgumentException if the arrays lengths do not match
or
> >>>       * there is insufficient data
> >>>       */
> >>> -    public double correlation(final double[] xArray, final double[] yArray)
throws IllegalArgumentException {
> >>> +    public static double correlation(final double[] xArray, final double[]
yArray)
> >>> +        throws IllegalArgumentException {
> >>>          SimpleRegression regression = new SimpleRegression();
> >>>          if (xArray.length != yArray.length) {
> >>>              throw new DimensionMismatchException(xArray.length, yArray.length);
> > As is, it is not clear (from a design point-of-view) that it would make
> > sense to override the method (hence the legitimate request to make it
> > static). If polymorphism is really the intention, I think that it would
> > be worth adding an interface ("Correlation" maybe?).
> 
> We have been talking about moving away from interfaces as the
> preferred way to support people plugging in alternative
> implementations because they have in several places gotten "behind"
> due to the fact that adding anything to them breaks compatibility. 
> We should probably continue that discussion in a different thread.

OK, comments kept for a separate thread...

> 
> The method above is the core of the implementation

One is left wondering why the "core" does not need anything else from the
class (no fields, no methods); it is self-sufficient (hence the legitimate
request for making it static).

> and making it
> static means that you can't extend this class using a different
> implementation.

Is that a problem (other than the practical one you invoke just below).

> It also breaks all existing code using the class.
> There are lots of other examples in [math] - throughout Commons
> actually - where methods *could* be static, but are not because we
> want to preserve the ability for ourselves or users to extend the
> classes and override the methods.

The issue which I'm raising is whether it is functionally clearer (as in
"How the class is _supposed_ to be used") to have (or not) a static method.
If the common case is inheritance (of which there is no example in CM) then
there must be an interface to single out the "core" functionality.
If instead, we focus on the specificity of the implementation, a user can
always "extend" the functionality by composition (i.e. his new class will
wrap a call to the static "PearsonsCorrelation.correlation" method).

> All of the evaluate() methods,
> for example, in the individual descriptive statistics, for example,
> could be static.  Convenience methods are provided in StatUtils to
> support static invocation.  I think a CorrelationUtils class for
> this, Covariance and Spearman's - which all have core implementation
> methods that "could" be static - is a better approach if we feel
> that we need to provide support for the procedural API style here.

That's not only an issue of providing procedural style API.


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message