COMPLEX-3: Make class immutable.
Project: http://git-wip-us.apache.org/repos/asf/commons-complex/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-complex/commit/b206d9c8 Tree: http://git-wip-us.apache.org/repos/asf/commons-complex/tree/b206d9c8 Diff: http://git-wip-us.apache.org/repos/asf/commons-complex/diff/b206d9c8 Branch: refs/heads/master Commit: b206d9c80c95433d530e639308e38b92c7e3fdef Parents: cd6412f Author: Gilles Sadowski <[email protected]> Authored: Sun Jan 8 03:53:58 2017 +0100 Committer: Gilles Sadowski <[email protected]> Committed: Sun Jan 8 04:23:40 2017 +0100 ---------------------------------------------------------------------- pom.xml | 7 + .../apache/commons/complex/RootsOfUnity.java | 201 ++++++------------- .../commons/complex/RootsOfUnityTest.java | 94 ++++----- 3 files changed, 105 insertions(+), 197 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-complex/blob/b206d9c8/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index e908f75..dbffb77 100644 --- a/pom.xml +++ b/pom.xml @@ -66,6 +66,13 @@ <version>4.11</version> <scope>test</scope> </dependency> + + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-math3</artifactId> + <version>3.6.1</version> + <scope>test</scope> + </dependency> </dependencies> <properties> http://git-wip-us.apache.org/repos/asf/commons-complex/blob/b206d9c8/src/main/java/org/apache/commons/complex/RootsOfUnity.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/complex/RootsOfUnity.java b/src/main/java/org/apache/commons/complex/RootsOfUnity.java index 8072f6c..7a06a69 100644 --- a/src/main/java/org/apache/commons/complex/RootsOfUnity.java +++ b/src/main/java/org/apache/commons/complex/RootsOfUnity.java @@ -16,186 +16,101 @@ */ package org.apache.commons.complex; -import java.io.Serializable; - /** - * A helper class for the computation and caching of the {@code n}-th roots of - * unity. - * - * @since 3.0 + * Computation of the {@code n}-th roots of unity. */ -public class RootsOfUnity implements Serializable { - - /** Serializable version id. */ - private static final long serialVersionUID = 20120201L; - - /** Error messages */ - private static String ROOTS_OF_UNITY_NOT_COMPUTED_YET = "Roots of Unity not computed yet"; - private static String CANNOT_COMPUTE_ZEROTH_ROOT_OF_UNITY = "Cannot compute zeroth root of unity"; - private static String OUT_OF_RANGE = "Value out of range"; - +public class RootsOfUnity { + /** 2 * π */ + private static final double TWO_PI = 2 * Math.PI; /** Number of roots of unity. */ - private int omegaCount; - - /** Real part of the roots. */ - private double[] omegaReal; - + private final int omegaCount; + /** The roots. */ + private final Complex[] omega; /** - * Imaginary part of the {@code n}-th roots of unity, for positive values - * of {@code n}. In this array, the roots are stored in counter-clockwise - * order. + * {@code true} if the constructor was called with a positive + * value of its argument {@code n}. */ - private double[] omegaImaginaryCounterClockwise; + private final boolean isCounterClockwise; /** - * Imaginary part of the {@code n}-th roots of unity, for negative values - * of {@code n}. In this array, the roots are stored in clockwise order. - */ - private double[] omegaImaginaryClockwise; - - /** - * {@code true} if {@link #computeRoots(int)} was called with a positive - * value of its argument {@code n}. In this case, counter-clockwise ordering - * of the roots of unity should be used. - */ - private boolean isCounterClockWise; - - /** - * Build an engine for computing the {@code n}-th roots of unity. - */ - public RootsOfUnity() { - - omegaCount = 0; - omegaReal = null; - omegaImaginaryCounterClockwise = null; - omegaImaginaryClockwise = null; - isCounterClockWise = true; - } - - /** - * Returns {@code true} if {@link #computeRoots(int)} was called with a - * positive value of its argument {@code n}. If {@code true}, then - * counter-clockwise ordering of the roots of unity should be used. + * Computes the {@code n}-th roots of unity. * - * @return {@code true} if the roots of unity are stored in - * counter-clockwise order - * @throws IllegalArgumentException if no roots of unity have been computed - * yet - */ - public synchronized boolean isCounterClockWise() { - - if (omegaCount == 0) { - throw new IllegalArgumentException(ROOTS_OF_UNITY_NOT_COMPUTED_YET); - } - return isCounterClockWise; - } - - /** - * <p> - * Computes the {@code n}-th roots of unity. The roots are stored in + * The roots are stored in an array * {@code omega[]}, such that {@code omega[k] = w ^ k}, where * {@code k = 0, ..., n - 1}, {@code w = exp(2 * pi * i / n)} and * {@code i = sqrt(-1)}. - * </p> - * <p> - * Note that {@code n} can be positive of negative - * </p> + * + * <p>{@code n} can be positive of negative ({@code abs(n)} is always + * the number of roots of unity):</p> * <ul> - * <li>{@code abs(n)} is always the number of roots of unity.</li> - * <li>If {@code n > 0}, then the roots are stored in counter-clockwise order.</li> - * <li>If {@code n < 0}, then the roots are stored in clockwise order.</p> + * <li>If {@code n > 0}, then the roots are stored in counter-clockwise order.</li> + * <li>If {@code n < 0}, then the roots are stored in clockwise order.</li> * </ul> * - * @param n the (signed) number of roots of unity to be computed - * @throws IllegalArgumentException if {@code n = 0} + * @param n The (signed) number of roots of unity to be computed. + * @throws IllegalArgumentException if {@code n == 0}? */ - public synchronized void computeRoots(int n) { - + public RootsOfUnity(int n) { if (n == 0) { - throw new IllegalArgumentException(CANNOT_COMPUTE_ZEROTH_ROOT_OF_UNITY); + throw new IllegalArgumentException("Zero-th root"); } - isCounterClockWise = n > 0; + omegaCount = Math.abs(n); + isCounterClockwise = n > 0; - // avoid repetitive calculations - final int absN = Math.abs(n); - - if (absN == omegaCount) { - return; - } - - // calculate everything from scratch - final double t = 2.0 * Math.PI / absN; + omega = new Complex[omegaCount]; + final double t = TWO_PI / omegaCount; final double cosT = Math.cos(t); final double sinT = Math.sin(t); - omegaReal = new double[absN]; - omegaImaginaryCounterClockwise = new double[absN]; - omegaImaginaryClockwise = new double[absN]; - omegaReal[0] = 1.0; - omegaImaginaryCounterClockwise[0] = 0.0; - omegaImaginaryClockwise[0] = 0.0; - for (int i = 1; i < absN; i++) { - omegaReal[i] = omegaReal[i - 1] * cosT - - omegaImaginaryCounterClockwise[i - 1] * sinT; - omegaImaginaryCounterClockwise[i] = omegaReal[i - 1] * sinT + - omegaImaginaryCounterClockwise[i - 1] * cosT; - omegaImaginaryClockwise[i] = -omegaImaginaryCounterClockwise[i]; + + double previousReal = 1; + double previousImag = 0; + omega[0] = new Complex(previousReal, previousImag); + for (int i = 1; i < omegaCount; i++) { + final double real = previousReal * cosT - previousImag * sinT; + final double imag = previousReal * sinT + previousImag * cosT; + + omega[i] = isCounterClockwise ? + new Complex(real, imag) : + new Complex(real, -imag); + + previousReal = real; + previousImag = imag; } - omegaCount = absN; } /** - * Get the real part of the {@code k}-th {@code n}-th root of unity. + * Returns {@code true} if {@link #RootsOfUnity(int)} was called with a + * positive value of its argument {@code n}. + * If {@code true}, then the imaginary parts of the successive roots are + * {@link #getRoot(int) returned} in counter-clockwise order. * - * @param k index of the {@code n}-th root of unity - * @return real part of the {@code k}-th {@code n}-th root of unity - * @throws IllegalArgumentException if no roots of unity have been - * computed yet - * @throws IllegalArgumentException if {@code k} is out of range + * @return {@code true} if the roots of unity are stored in + * counter-clockwise order. */ - public synchronized double getReal(int k) { - - if (omegaCount == 0) { - throw new IllegalArgumentException(ROOTS_OF_UNITY_NOT_COMPUTED_YET); - } - if ((k < 0) || (k >= omegaCount)) { - throw new IllegalArgumentException(OUT_OF_RANGE); - } - - return omegaReal[k]; + public boolean isCounterClockwise() { + return isCounterClockwise; } /** - * Get the imaginary part of the {@code k}-th {@code n}-th root of unity. + * Gets the {@code k}-th among the computed roots of unity. * - * @param k index of the {@code n}-th root of unity - * @return imaginary part of the {@code k}-th {@code n}-th root of unity - * @throws IllegalArgumentException if no roots of unity have been - * computed yet - * @throws IllegalArgumentException if {@code k} is out of range + * @param k Index of the requested value. + * @return the {@code k}-th among the {@code N}-th root of unity + * where {@code N} is the absolute value of the argument passed + * to the constructor. + * @throws IndexOutOfBoundsException if {@code k} is out of range. */ - public synchronized double getImaginary(int k) { - - if (omegaCount == 0) { - throw new IllegalArgumentException(ROOTS_OF_UNITY_NOT_COMPUTED_YET); - } - if ((k < 0) || (k >= omegaCount)) { - throw new IllegalArgumentException(OUT_OF_RANGE); - } - - return isCounterClockWise ? omegaImaginaryCounterClockwise[k] : - omegaImaginaryClockwise[k]; + public Complex getRoot(int k) { + return omega[k]; } /** - * Returns the number of roots of unity currently stored. If - * {@link #computeRoots(int)} was called with {@code n}, then this method - * returns {@code abs(n)}. If no roots of unity have been computed yet, this - * method returns 0. + * Gets the number of roots of unity. * - * @return the number of roots of unity currently stored + * @return the number of roots of unity. */ - public synchronized int getNumberOfRoots() { + public int getNumberOfRoots() { return omegaCount; } } http://git-wip-us.apache.org/repos/asf/commons-complex/blob/b206d9c8/src/test/java/org/apache/commons/complex/RootsOfUnityTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/complex/RootsOfUnityTest.java b/src/test/java/org/apache/commons/complex/RootsOfUnityTest.java index 81ab1a3..642c87a 100644 --- a/src/test/java/org/apache/commons/complex/RootsOfUnityTest.java +++ b/src/test/java/org/apache/commons/complex/RootsOfUnityTest.java @@ -25,74 +25,60 @@ import org.junit.Test; * */ public class RootsOfUnityTest { - @Test(expected = IllegalArgumentException.class) - public void testMathIllegalArgument1() { - final RootsOfUnity roots = new RootsOfUnity(); - roots.getReal(0); + public void testPrecondition() { + new RootsOfUnity(0); } - - @Test(expected = IllegalArgumentException.class) - public void testMathIllegalArgument2() { - final RootsOfUnity roots = new RootsOfUnity(); - roots.getImaginary(0); + @Test(expected = IndexOutOfBoundsException.class) + public void testGetRootPrecondition1() { + final int n = 3; + final RootsOfUnity roots = new RootsOfUnity(n); + roots.getRoot(-1); } - - @Test(expected = IllegalArgumentException.class) - public void testMathIllegalArgument3() { - final RootsOfUnity roots = new RootsOfUnity(); - roots.isCounterClockWise(); + @Test(expected = IndexOutOfBoundsException.class) + public void testGetRootPrecondition2() { + final int n = -2; + final RootsOfUnity roots = new RootsOfUnity(n); + roots.getRoot(2); } - @Test(expected = IllegalArgumentException.class) - public void testZeroNumberOfRoots() { - final RootsOfUnity roots = new RootsOfUnity(); - roots.computeRoots(0); + @Test + public void testGetNumberOfRoots1() { + final int n = 5; + final RootsOfUnity roots = new RootsOfUnity(n); + Assert.assertEquals(n, roots.getNumberOfRoots()); + Assert.assertTrue(roots.isCounterClockwise()); } - @Test - public void testGetNumberOfRoots() { - final RootsOfUnity roots = new RootsOfUnity(); - Assert.assertEquals("", 0, roots.getNumberOfRoots()); - roots.computeRoots(5); - Assert.assertEquals("", 5, roots.getNumberOfRoots()); - /* - * Testing -5 right after 5 is important, as the roots in this case are - * not recomputed. - */ - roots.computeRoots(-5); - Assert.assertEquals("", 5, roots.getNumberOfRoots()); - roots.computeRoots(6); - Assert.assertEquals("", 6, roots.getNumberOfRoots()); + public void testGetNumberOfRoots2() { + final int n = -4; + final RootsOfUnity roots = new RootsOfUnity(n); + Assert.assertEquals(Math.abs(n), roots.getNumberOfRoots()); + Assert.assertFalse(roots.isCounterClockwise()); } @Test public void testComputeRoots() { - final RootsOfUnity roots = new RootsOfUnity(); + final double tol = Math.ulp(1d); + final org.apache.commons.math3.complex.RootsOfUnity cmRoots = + new org.apache.commons.math3.complex.RootsOfUnity(); for (int n = -10; n < 11; n++) { - /* - * Testing -n right after n is important, as the roots in this case - * are not recomputed. - */ + final int absN = Math.abs(n); if (n != 0) { - roots.computeRoots(n); - doTestComputeRoots(roots); - roots.computeRoots(-n); - doTestComputeRoots(roots); + cmRoots.computeRoots(n); + final RootsOfUnity roots = new RootsOfUnity(n); + for (int k = 0; k < absN; k++) { + final Complex z = roots.getRoot(k); + Assert.assertEquals("n=" + n + " k=" + k, + cmRoots.getReal(k), + z.getReal(), + tol); + Assert.assertEquals("n=" + n + " k=" + k, + cmRoots.getImaginary(k), + z.getImaginary(), + tol); + } } } } - - private void doTestComputeRoots(final RootsOfUnity roots) { - final int n = roots.isCounterClockWise() ? roots.getNumberOfRoots() : - -roots.getNumberOfRoots(); - final double tol = 10 * Math.ulp(1.0); - for (int k = 0; k < n; k++) { - final double t = 2.0 * Math.PI * k / n; - @SuppressWarnings("boxing") - final String msg = String.format("n = %d, k = %d", n, k); - Assert.assertEquals(msg, Math.cos(t), roots.getReal(k), tol); - Assert.assertEquals(msg, Math.sin(t), roots.getImaginary(k), tol); - } - } }
