Fixed few bugs and updated JUnit

Author: avbasak1

commons-math-ga/src/main/java/org/apache/commons/math4/ga/AbstractGeneticAlgorithm.java

@@ -47,7 +47,7 @@ public abstract class AbstractGeneticAlgorithm<P> {
      */
     private int generationsEvolved;
 
-    /** The elitism rate haveing default value of .25. */
+    /** The elitism rate having default value of .25. */
     private double elitismRate = .25;
 
     /**

commons-math-ga/src/main/java/org/apache/commons/math4/ga/GeneticAlgorithm.java

@@ -52,12 +52,7 @@ public GeneticAlgorithm(final CrossoverPolicy<P> crossoverPolicy, final double c
             final SelectionPolicy<P> selectionPolicy) {
         super(crossoverPolicy, mutationPolicy, selectionPolicy);
 
-        if (crossoverRate < 0 || crossoverRate > 1) {
-            throw new GeneticException(GeneticException.OUT_OF_RANGE, crossoverRate, Constants.CROSSOVER_RATE, 0, 1);
-        }
-        if (mutationRate < 0 || mutationRate > 1) {
-            throw new GeneticException(GeneticException.OUT_OF_RANGE, mutationRate, Constants.MUTATION_RATE, 0, 1);
-        }
+        checkValidity(crossoverRate, mutationRate);
         this.crossoverRate = crossoverRate;
         this.mutationRate = mutationRate;
     }
@@ -76,32 +71,38 @@ public GeneticAlgorithm(final CrossoverPolicy<P> crossoverPolicy, final double c
             final double elitismRate) {
         super(crossoverPolicy, mutationPolicy, selectionPolicy, elitismRate);
 
-        if (crossoverRate < 0 || crossoverRate > 1) {
-            throw new GeneticException(GeneticException.OUT_OF_RANGE, crossoverRate, Constants.CROSSOVER_RATE, 0, 1);
-        }
-        if (mutationRate < 0 || mutationRate > 1) {
-            throw new GeneticException(GeneticException.OUT_OF_RANGE, mutationRate, Constants.MUTATION_RATE, 0, 1);
-        }
+        checkValidity(crossoverRate, mutationRate);
         this.crossoverRate = crossoverRate;
         this.mutationRate = mutationRate;
     }
 
+    private void checkValidity(final double crossoverRateInput, final double inputMutationRate) {
+        if (crossoverRateInput < 0 || crossoverRateInput > 1) {
+            throw new GeneticException(GeneticException.OUT_OF_RANGE, crossoverRateInput, Constants.CROSSOVER_RATE, 0,
+                    1);
+        }
+        if (inputMutationRate < 0 || inputMutationRate > 1) {
+            throw new GeneticException(GeneticException.OUT_OF_RANGE, inputMutationRate, Constants.MUTATION_RATE, 0, 1);
+        }
+    }
+
     /**
      * Evolve the given population into the next generation.
      * <ol>
-     *  <li>Get nextGeneration population to fill from <code>current</code>
-     *      generation, using its nextGeneration method</li>
-     *  <li>Loop until new generation is filled:
-     *  <ul><li>Apply configured SelectionPolicy to select a pair of parents
-     *          from <code>current</code></li>
-     *      <li>With probability = {@link #getCrossoverRate()}, apply
-     *          configured {@link CrossoverPolicy} to parents</li>
-     *      <li>With probability = {@link #getMutationRate()}, apply
-     *          configured {@link MutationPolicy} to each of the offspring</li>
-     *      <li>Add offspring individually to nextGeneration,
-     *          space permitting</li>
-     *  </ul></li>
-     *  <li>Return nextGeneration</li>
+     * <li>Get nextGeneration population to fill from <code>current</code>
+     * generation, using its nextGeneration method</li>
+     * <li>Loop until new generation is filled:
+     * <ul>
+     * <li>Apply configured SelectionPolicy to select a pair of parents from
+     * <code>current</code></li>
+     * <li>With probability = {@link #getCrossoverRate()}, apply configured
+     * {@link CrossoverPolicy} to parents</li>
+     * <li>With probability = {@link #getMutationRate()}, apply configured
+     * {@link MutationPolicy} to each of the offspring</li>
+     * <li>Add offspring individually to nextGeneration, space permitting</li>
+     * </ul>
+     * </li>
+     * <li>Return nextGeneration</li>
      * </ol>
      *
      * @param current the current population.

commons-math-ga/src/main/java/org/apache/commons/math4/ga/chromosome/IntegralValuedChromosome.java

@@ -22,6 +22,7 @@
 import org.apache.commons.math4.ga.exception.GeneticException;
 import org.apache.commons.math4.ga.fitness.FitnessFunction;
 import org.apache.commons.math4.ga.utils.ChromosomeRepresentationUtils;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * Chromosome represented by a list of integral values. The acceptable integral
@@ -89,9 +90,7 @@ public int getMax() {
      * Asserts that <code>representation</code> can represent a valid chromosome.
      */
     private void checkValidity() {
-        if (min > max) {
-            throw new GeneticException(GeneticException.TOO_LARGE, min, max);
-        }
+        ValidationUtils.checkForMinMax(min, max);
         for (int i : getRepresentation()) {
             if (i < min || i >= max) {
                 throw new GeneticException(GeneticException.ILLEGAL_ARGUMENT, i);

commons-math-ga/src/main/java/org/apache/commons/math4/ga/chromosome/RealValuedChromosome.java

@@ -23,6 +23,7 @@
 import org.apache.commons.math4.ga.exception.GeneticException;
 import org.apache.commons.math4.ga.fitness.FitnessFunction;
 import org.apache.commons.math4.ga.utils.ChromosomeRepresentationUtils;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * DoubleEncodedChromosome is used for representing chromosome encoded as
@@ -114,9 +115,7 @@ public double getMax() {
      * Asserts that <code>representation</code> can represent a valid chromosome.
      */
     private void checkValidity() {
-        if (min > max) {
-            throw new GeneticException(GeneticException.TOO_LARGE, min, max);
-        }
+        ValidationUtils.checkForMinMax(min, max);
         for (double i : getRepresentation()) {
             if (i < min || i >= max) {
                 throw new GeneticException(GeneticException.ILLEGAL_ARGUMENT, i);

commons-math-ga/src/main/java/org/apache/commons/math4/ga/decoder/RandomKeyDecoder.java

@@ -53,19 +53,19 @@ protected List<U> decode(AbstractListChromosome<Double, List<U>> chromosome) {
         final List<Double> sortedRepresentation = new ArrayList<>(representation);
         Collections.sort(sortedRepresentation);
 
-        final int l = baseSequence.size();
+        final int sequenceLength = baseSequence.size();
 
         // the size of the three lists must be equal
-        if (representation.size() != l) {
-            throw new GeneticException(GeneticException.SIZE_MISMATCH, representation.size(), l);
+        if (representation.size() != sequenceLength) {
+            throw new GeneticException(GeneticException.SIZE_MISMATCH, representation.size(), sequenceLength);
         }
 
         // do not modify the original representation
         final List<Double> representationCopy = new ArrayList<>(representation);
 
         // now find the indices in the original repr and use them for permuting
-        final List<U> res = new ArrayList<>(l);
-        for (int i = 0; i < l; i++) {
+        final List<U> res = new ArrayList<>(sequenceLength);
+        for (int i = 0; i < sequenceLength; i++) {
             final int index = representationCopy.indexOf(sortedRepresentation.get(i));
             res.add(baseSequence.get(index));
             representationCopy.set(index, null);

commons-math-ga/src/main/java/org/apache/commons/math4/ga/listener/ConvergenceListenerRegistry.java

@@ -21,6 +21,7 @@
 import java.util.List;
 
 import org.apache.commons.math4.ga.population.Population;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * This class is the default implementation of ConvergenceListenerRegistry. It
@@ -72,10 +73,9 @@ public void notifyAll(int generation, Population<P> population) {
      * @param convergenceListeners list of {@link ConvergenceListener}
      */
     public void addConvergenceListeners(List<ConvergenceListener<P>> convergenceListeners) {
-        if (convergenceListeners != null) {
-            for (ConvergenceListener<P> convergenceListener : convergenceListeners) {
-                this.listeners.add(convergenceListener);
-            }
+        ValidationUtils.checkForNull("Null convergenceListeners", convergenceListeners);
+        for (ConvergenceListener<P> convergenceListener : convergenceListeners) {
+            addConvergenceListener(convergenceListener);
         }
     }
 

commons-math-ga/src/main/java/org/apache/commons/math4/ga/mutation/BinaryMutation.java

@@ -25,13 +25,18 @@
  * @param <P> phenotype of chromosome
  * @since 4.0
  */
-public class BinaryMutation<P> extends AbstractListChromosomeMutationPolicy<Integer, P> {
+public class BinaryMutation<P> extends IntegralValuedMutation<P> {
+
+    public BinaryMutation() {
+        super(0, 2);
+    }
 
     /**
      * {@inheritDoc}
      */
     @Override
     protected void checkValidity(Chromosome<P> original) {
+        super.checkValidity(original);
         if (!BinaryChromosome.class.isAssignableFrom(original.getClass())) {
             throw new GeneticException(GeneticException.ILLEGAL_ARGUMENT, original.getClass().getSimpleName());
         }

commons-math-ga/src/main/java/org/apache/commons/math4/ga/mutation/IntegralValuedMutation.java

@@ -20,6 +20,7 @@
 import org.apache.commons.math4.ga.chromosome.IntegralValuedChromosome;
 import org.apache.commons.math4.ga.exception.GeneticException;
 import org.apache.commons.math4.ga.utils.RandomGenerator;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * Mutation for {@link IntegralValuedChromosome}. Randomly changes few genes.
@@ -42,9 +43,7 @@ public class IntegralValuedMutation<P> extends AbstractListChromosomeMutationPol
     public IntegralValuedMutation(final int min, final int max) {
         this.min = min;
         this.max = max;
-        if (min > max) {
-            throw new GeneticException(GeneticException.TOO_LARGE, min, max);
-        }
+        ValidationUtils.checkForMinMax(min, max);
     }
 
     /**
@@ -83,7 +82,7 @@ protected void checkValidity(Chromosome<P> original) {
      */
     @Override
     protected Integer mutateGene(Integer originalValue) {
-        return RandomGenerator.getRandomGenerator().nextInt();
+        return min + RandomGenerator.getRandomGenerator().nextInt(max - min);
     }
 
 }

commons-math-ga/src/main/java/org/apache/commons/math4/ga/mutation/RealValuedMutation.java

@@ -20,6 +20,7 @@
 import org.apache.commons.math4.ga.chromosome.RealValuedChromosome;
 import org.apache.commons.math4.ga.exception.GeneticException;
 import org.apache.commons.math4.ga.utils.RandomGenerator;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * This class mutates real-valued chromosome.
@@ -50,10 +51,7 @@ public RealValuedMutation() {
     public RealValuedMutation(double min, double max) {
         this.min = min;
         this.max = max;
-
-        if (min > max) {
-            throw new GeneticException(GeneticException.TOO_LARGE, min, max);
-        }
+        ValidationUtils.checkForMinMax(min, max);
     }
 
     /**

commons-math-ga/src/main/java/org/apache/commons/math4/ga/population/ListPopulation.java

@@ -25,6 +25,7 @@
 import org.apache.commons.math4.ga.chromosome.Chromosome;
 import org.apache.commons.math4.ga.exception.GeneticException;
 import org.apache.commons.math4.ga.utils.Constants;
+import org.apache.commons.math4.ga.utils.ValidationUtils;
 
 /**
  * Population of chromosomes represented by a {@link List}.
@@ -60,9 +61,8 @@ public ListPopulation(final int populationLimit) {
      */
     public ListPopulation(final List<Chromosome<P>> chromosomes, final int populationLimit) {
 
-        if (chromosomes == null) {
-            throw new GeneticException(GeneticException.NULL_ARGUMENT, "chromosomes");
-        }
+        ValidationUtils.checkForNull("chromosomes", chromosomes);
+
         if (populationLimit <= 0) {
             throw new GeneticException(GeneticException.NOT_STRICTLY_POSITIVE, populationLimit);
         }
@@ -195,7 +195,7 @@ public Iterator<Chromosome<P>> iterator() {
     public Population<P> nextGeneration(final double elitismRate) {
         final List<Chromosome<P>> oldChromosomes = getChromosomeList();
 
-        if (oldChromosomes.size() * elitismRate == 0) {
+        if ((int) (oldChromosomes.size() * elitismRate) == 0) {
             // if no of elite chromosome is 0 crete and return an empty population instance.
             return new ListPopulation<>(getPopulationLimit());
         } else {