Code cleanup: moved all computations to the constructor, allowing to make

the class immutable.


git-svn-id: https://svn.apache.org/repos/asf/commons/proper/math/trunk@1374308 13f79535-47bb-0310-9956-ffa450edef68

src/main/java/org/apache/commons/math3/optimization/fitting/HarmonicFitter.java

@@ -177,20 +177,21 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
      * number of measurements.</p>
      */
     public static class ParameterGuesser {
-        /** Sampled observations. */
-        private final WeightedObservedPoint[] observations;
         /** Amplitude. */
-        private double a;
+        private final double a;
         /** Angular frequency. */
-        private double omega;
+        private final double omega;
         /** Phase. */
-        private double phi;
+        private final double phi;
 
         /**
          * Simple constructor.
-         * @param observations sampled observations
+         *
-         * @throws NumberIsTooSmallException if the sample is too short or if
+         * @param observations Sampled observations.
-         * the first guess cannot be computed.
+         * @throws NumberIsTooSmallException if the sample is too short.
+         * @throws ZeroException if the abscissa range is zero.
+         * @throws MathIllegalStateException when the guessing procedure cannot
+         * produce sensible results.
          */
         public ParameterGuesser(WeightedObservedPoint[] observations) {
             if (observations.length < 4) {
@@ -198,7 +199,13 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                                                     observations.length, 4, true);
             }
 
-            this.observations = observations.clone();
+            final WeightedObservedPoint[] sorted = sortObservations(observations);
+
+            final double aOmega[] = guessAOmega(sorted);
+            a = aOmega[0];
+            omega = aOmega[1];
+
+            phi = guessPhi(sorted);
         }
 
         /**
@@ -212,16 +219,18 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
          * </ul>
          */
         public double[] guess() {
-            sortObservations();
-            guessAOmega();
-            guessPhi();
             return new double[] { a, omega, phi };
         }
 
         /**
          * Sort the observations with respect to the abscissa.
+         *
+         * @param unsorted Input observations.
+         * @return the input observations, sorted.
          */
-        private void sortObservations() {
+        private WeightedObservedPoint[] sortObservations(WeightedObservedPoint[] unsorted) {
+            final WeightedObservedPoint[] observations = unsorted.clone();
+
             // Since the samples are almost always already sorted, this
             // method is implemented as an insertion sort that reorders the
             // elements in place. Insertion sort is very efficient in this case.
@@ -243,6 +252,8 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                     curr = observations[j];
                 }
             }
+
+            return observations;
         }
 
         /**
@@ -250,11 +261,16 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
          * This method assumes that the {@link #sortObservations()} method
          * has been called previously.
          *
+         * @param observations Observations, sorted w.r.t. abscissa.
          * @throws ZeroException if the abscissa range is zero.
          * @throws MathIllegalStateException when the guessing procedure cannot
          * produce sensible results.
+         * @return the guessed amplitude (at index 0) and circular frequency
+         * (at index 1).
          */
-        private void guessAOmega() {
+        private double[] guessAOmega(WeightedObservedPoint[] observations) {
+            final double[] aOmega = new double[2];
+
             // initialize the sums for the linear model between the two integrals
             double sx2 = 0;
             double sy2 = 0;
@@ -305,7 +321,7 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                 if (xRange == 0) {
                     throw new ZeroException();
                 }
-                omega = 2 * Math.PI / xRange;
+                aOmega[1] = 2 * Math.PI / xRange;
 
                 double yMin = Double.POSITIVE_INFINITY;
                 double yMax = Double.NEGATIVE_INFINITY;
@@ -318,7 +334,7 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                         yMax = y;
                     }
                 }
-                a = 0.5 * (yMax - yMin);
+                aOmega[0] = 0.5 * (yMax - yMin);
             } else {
                 if (c2 == 0) {
                     // In some ill-conditioned cases (cf. MATH-844), the guesser
@@ -326,15 +342,20 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                     throw new MathIllegalStateException(LocalizedFormats.ZERO_DENOMINATOR);
                 }
 
-                a = FastMath.sqrt(c1 / c2);
+                aOmega[0] = FastMath.sqrt(c1 / c2);
-                omega = FastMath.sqrt(c2 / c3);
+                aOmega[1] = FastMath.sqrt(c2 / c3);
             }
+
+            return aOmega;
         }
 
         /**
          * Estimate a first guess of the phase.
+         *
+         * @param observations Observations, sorted w.r.t. abscissa.
+         * @return the guessed phase.
          */
-        private void guessPhi() {
+        private double guessPhi(WeightedObservedPoint[] observations) {
             // initialize the means
             double fcMean = 0;
             double fsMean = 0;
@@ -356,7 +377,7 @@ public class HarmonicFitter extends CurveFitter<HarmonicOscillator.Parametric> {
                 fsMean += omega * currentY * sine + currentYPrime * cosine;
             }
 
-            phi = FastMath.atan2(-fsMean, fcMean);
+            return FastMath.atan2(-fsMean, fcMean);
         }
     }
 }

src/test/java/org/apache/commons/math3/optimization/fitting/HarmonicFitterTest.java

@@ -191,14 +191,12 @@ public class HarmonicFitterTest {
             points[i] = new WeightedObservedPoint(1, i, y[i]);
         }
 
-        final HarmonicFitter.ParameterGuesser guesser
-            = new HarmonicFitter.ParameterGuesser(points);
-
         // The guesser fails because the function is far from an harmonic
         // function: It is a triangular periodic function with amplitude 3
         // and period 12, and all sample points are taken at integer abscissae
         // so function values all belong to the integer subset {-3, -2, -1, 0,
         // 1, 2, 3}.
-        guesser.guess();
+        final HarmonicFitter.ParameterGuesser guesser
+            = new HarmonicFitter.ParameterGuesser(points);
     }
 }