On 3/29/11 4:43 AM, Gilles Sadowski wrote: > On Sun, Mar 27, 2011 at 05:25:59PM -0700, Phil Steitz wrote: >> I would prefer not to make that method static, since that means it >> cannot be overridden and it is used internally in this class as well >> as Spearman's. If we want to provide a convenience method with >> static signature, we should add it to StatUtils or create a >> CorrelationUtils class to bundle static methods such as this and >> similar for Covariance, Spearman's etc. >> >> Phil >> >> On 3/27/11 3:06 PM, er...@apache.org wrote: >>> Author: erans >>> Date: Sun Mar 27 22:06:18 2011 >>> New Revision: 1086057 >>> >>> URL: http://svn.apache.org/viewvc?rev=1086057&view=rev >>> Log: >>> MATH-549 >>> Method can be "static". >>> >>> Modified: >>> >>> commons/proper/math/trunk/src/main/java/org/apache/commons/math/stat/correlation/PearsonsCorrelation.java >>> >>> commons/proper/math/trunk/src/test/java/org/apache/commons/math/stat/correlation/PearsonsCorrelationTest.java >>> >>> 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. The method above is the core of the implementation and making it static means that you can't extend this class using a different implementation. 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. 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. Phil > > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
