cleanups

Author: mtf90

CHANGELOG.md

@@ -61,6 +61,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * `Minimizer` no longer provides a `getInstance()` method but can be instantiated directly.
 * The `OneSEVPA` interface has been generalized to an arbitrary (k-)`SEVPA` interface. The old `OneSEVPA` specialization is still available and unchanged.
 * The `OneSEVPAUtils` class has been merged into the `OneSEVPAs` class. 
+* The `RandomUtil` class has been made a factory (non-instantiable, only static methods) and its methods now require the random object as first parameter.
 * AutomataLib classes no longer implement `Serializable`. We never fully supported the semantics of the interface and never intended to do so. In fact, the old approach failed miserably if any class was involved where we missed an "implements Serializable" statement. In order to prevent confusion by promising false contracts, implementing this markup interface has been removed. Serialization should now be done in user-land via one of the many external (and more optimizable) serialization frameworks such as FST, XStream, etc.
 * `ShortestPaths` now offers fewer but less confusing methods. Previously there were methods such as `shortestPath` that took an initial node and multiple target nodes which much better fits to the idea of computing `shortestPath*s*` rather than any shortest path to one of the target nodes. The old behavior can still be replicated with the generic `Predicate`-based versions.
 * `StrictPriorityQueue` is now package-private as it is only meant for internal use.
@@ -73,6 +74,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Removed `AbstractCompactNPGraph`, use `AbstractCompactGraph` instead.
 * Removed `AbstractCompactSimpleGraph`. All functionality is provided in `CompactSimpleGraph`.
 * Removed `CmpUtil#safeComparator`. Use `Comparators#nullsFirst` or `Comparators#nullsLast` instead.
+* Removed `DelegateVisualizationHelper` without replacement. Instead, directly override/extend the `VisualizationHelper` you want to delegate to.
 * Removed the DFS-specific `DFSTraversalVisitor` (and related classes) without replacement. Client-code that relied on this class can re-implement the functionality by providing an own implementation of the more general `GraphTraversalVisitor`. See the changes on the `DFSExample` for reference.
 * Removed (unused) `DisjointSetForestInt` class without replacement.
 * Removed non-static methods on `RandomAutomata` factory (including the `getInstance()` method).

api/src/main/java/net/automatalib/graph/helper/SimpleNodeIDs.java

@@ -52,6 +52,9 @@ public int getNodeId(N node) {
 
     @Override
     public N getNode(int id) {
+        if (id < 0 || id >= nodes.size()) {
+            throw new IllegalArgumentException();
+        }
         return nodes.get(id);
     }
 }

api/src/main/java/net/automatalib/visualization/helper/DelegateVisualizationHelper.java

@@ -15,7 +15,7 @@
  */
 
 /**
- * This package (including sub-packages) contains several approaches for the visualization of the different types of
- * automata, transition systems, and graphs supported by AutomataLib.
+ * This package contains several approaches for the visualization of the different types of automata, transition
+ * systems, and graphs supported by AutomataLib.
  */
 package net.automatalib.visualization;

api/src/main/java/net/automatalib/visualization/package-info.java

@@ -23,23 +23,13 @@
 
 import org.checkerframework.checker.nullness.qual.Nullable;
 
-public class RandomUtil {
+public final class RandomUtil {
 
-    private final Random random;
-
-    public RandomUtil() {
-        this(new Random());
-    }
-
-    public RandomUtil(Random random) {
-        this.random = random;
+    private RandomUtil() {
+        // prevent instantiation
     }
 
-    public <T> @Nullable T choose(T[] array) {
-        return choose(array, random);
-    }
-
-    public static <T> @Nullable T choose(T[] array, Random rand) {
+    public static <T> @Nullable T choose(Random rand, T[] array) {
         int len = array.length;
         if (len == 0) {
             return null;
@@ -48,11 +38,7 @@ public RandomUtil(Random random) {
         return array[idx];
     }
 
-    public <T> @Nullable T choose(List<? extends T> list) {
-        return choose(list, random);
-    }
-
-    public static <T> @Nullable T choose(List<? extends T> list, Random rand) {
+    public static <T> @Nullable T choose(Random rand, List<? extends T> list) {
         int size = list.size();
         if (size == 0) {
             return null;
@@ -61,49 +47,25 @@ public RandomUtil(Random random) {
         return list.get(idx);
     }
 
-    public Random getRandom() {
-        return random;
-    }
-
-    /**
-     * Sample a specified number of distinct integers from a specified range.
-     * <p>
-     * The implementation is based on Floyd's <a href="https://doi.org/10.1145/30401.315746">Algorithm F2</a>. Note that
-     * this algorithm ensures equal probability of each integer within in the specified range to appear in the returned
-     * array but no equal probability of their order.
-     *
-     * @param num
-     *         number of integers to sample
-     * @param min
-     *         lower bound (inclusive) of sampled values
-     * @param max
-     *         upper bound (exclusive) of samples values
-     *
-     * @return an array of distinct integers sampled from the specified range
-     */
-    public int[] distinctIntegers(int num, int min, int max) {
-        return distinctIntegers(num, min, max, random);
-    }
-
     /**
      * Sample a specified number of distinct integers from a specified range.
      * <p>
      * The implementation is based on Floyd's <a href="https://doi.org/10.1145/30401.315746">Algorithm F2</a>. Note that
      * this algorithm ensures equal probability of each integer within in the specified range to appear in the returned
      * array but no equal probability of their order.
      *
+     * @param rand
+     *         the random instance for generating numbers
      * @param num
      *         number of integers to sample
      * @param min
      *         lower bound (inclusive) of sampled values
      * @param max
      *         upper bound (exclusive) of samples values
-     * @param rand
-     *         the random instance for generating numbers
      *
      * @return an array of distinct integers sampled from the specified range
      */
-    public static int[] distinctIntegers(int num, int min, int max, Random rand) {
+    public static int[] distinctIntegers(Random rand, int num, int min, int max) {
         int range = max - min;
         int size = Math.min(num, range);
 
@@ -122,19 +84,11 @@ public static int[] distinctIntegers(int num, int min, int max, Random rand) {
         return result;
     }
 
-    public int[] distinctIntegers(int num, int max) {
-        return distinctIntegers(num, max, random);
-    }
-
-    public static int[] distinctIntegers(int num, int max, Random rand) {
-        return distinctIntegers(num, 0, max, rand);
-    }
-
-    public <T> List<T> sample(List<? extends T> list, int num) {
-        return sample(list, num, random);
+    public static int[] distinctIntegers(Random rand, int num, int max) {
+        return distinctIntegers(rand, num, 0, max);
     }
 
-    public static <T> List<T> sample(List<? extends T> list, int num, Random rand) {
+    public static <T> List<T> sample(Random rand, List<? extends T> list, int num) {
         if (list.isEmpty()) {
             return Collections.emptyList();
         }
@@ -155,41 +109,23 @@ public static <T> List<T> sample(List<? extends T> list, int num, Random rand) {
      * this algorithm ensures equal probability of each element within in the specified list to appear in the returned
      * list but no equal probability of their order.
      *
+     * @param rand
+     *         the random instance for generating numbers
      * @param list
      *         the list to sample elements from
      * @param num
      *         number of integers to sample
      *
      * @return a list of distinct elements sampled from the specified list
      */
-    public <T> List<T> sampleUnique(List<? extends T> list, int num) {
-        return sampleUnique(list, num, random);
-    }
-
-    /**
-     * Sample a specified number of elements from specified list.
-     * <p>
-     * The implementation is based on Floyd's <a href="https://doi.org/10.1145/30401.315746">Algorithm F2</a>. Note that
-     * this algorithm ensures equal probability of each element within in the specified list to appear in the returned
-     * list but no equal probability of their order.
-     *
-     * @param list
-     *         the list to sample elements from
-     * @param num
-     *         number of integers to sample
-     * @param rand
-     *         the random instance for generating numbers
-     *
-     * @return a list of distinct elements sampled from the specified list
-     */
-    public static <T> List<T> sampleUnique(List<? extends T> list, int num, Random rand) {
+    public static <T> List<T> sampleUnique(Random rand, List<? extends T> list, int num) {
         int elems = list.size();
 
         if (elems == 0) {
             return Collections.emptyList();
         }
 
-        int[] indices = distinctIntegers(num, elems, rand);
+        int[] indices = distinctIntegers(rand, num, elems);
         List<T> result = new ArrayList<>(indices.length);
 
         for (int index : indices) {

commons/util/src/main/java/net/automatalib/common/util/random/RandomUtil.java

@@ -31,93 +31,91 @@ public class RandomUtilTest {
 
     private static final int HIGH = 10;
 
-    private RandomUtil util;
-
+    private Random random;
     private List<Integer> list;
     private Integer[] array;
 
     @BeforeClass
     public void setUp() {
-        this.util = new RandomUtil(new Random(42));
+        this.random = new Random(42);
         this.list = CollectionsUtil.intRange(0, HIGH, 1);
         this.array = IntStream.range(0, HIGH).boxed().toArray(Integer[]::new);
     }
 
     @Test
     public void testChooseArray() {
-        Assert.assertNull(util.choose(new Object[0]));
-        Assert.assertEquals(util.choose(new Object[] {1}), 1);
+        Assert.assertNull(RandomUtil.choose(random, new Object[0]));
+        Assert.assertEquals(RandomUtil.choose(random, new Object[] {1}), 1);
 
-        final Integer chosenElement = util.choose(array);
+        final Integer chosenElement = RandomUtil.choose(random, array);
         Assert.assertNotNull(chosenElement);
         Assert.assertTrue(0 <= chosenElement && chosenElement < HIGH);
     }
 
     @Test
     public void testChooseList() {
-        Assert.assertNull(util.choose(Collections.emptyList()));
-        Assert.assertEquals(util.choose(Collections.singletonList(1)), (Integer) 1);
+        Assert.assertNull(RandomUtil.choose(random, Collections.emptyList()));
+        Assert.assertEquals(RandomUtil.choose(random, Collections.singletonList(1)), (Integer) 1);
 
-        final Integer chosenElement = util.choose(list);
+        final Integer chosenElement = RandomUtil.choose(random, list);
         Assert.assertNotNull(chosenElement);
         Assert.assertTrue(0 <= chosenElement && chosenElement < HIGH);
     }
 
     @Test
     public void testDistinctIntegers() {
-        Assert.assertEquals(util.distinctIntegers(0, HIGH).length, 0);
+        Assert.assertEquals(RandomUtil.distinctIntegers(random, 0, HIGH).length, 0);
 
-        int[] result = util.distinctIntegers(HIGH, HIGH);
-        util.distinctIntegers(HIGH, HIGH);
+        int[] result = RandomUtil.distinctIntegers(random, HIGH, HIGH);
         Assert.assertEquals(result.length, HIGH);
         Arrays.sort(result);
         Assert.assertEquals(box(result), array);
 
         // Sample from lower half domain
-        result = util.distinctIntegers(HIGH, HIGH / 2);
+        result = RandomUtil.distinctIntegers(random, HIGH, HIGH / 2);
         Assert.assertEquals(result.length, HIGH / 2);
         Arrays.sort(result);
         Assert.assertEquals(box(result), Arrays.copyOfRange(array, 0, HIGH / 2));
 
         // Sample from upper half domain
-        result = util.distinctIntegers(HIGH, HIGH / 2, HIGH);
+        result = RandomUtil.distinctIntegers(random, HIGH, HIGH / 2, HIGH);
         Assert.assertEquals(result.length, HIGH / 2);
         Arrays.sort(result);
         Assert.assertEquals(box(result), Arrays.copyOfRange(array, HIGH / 2, HIGH));
 
         // Sample all but one from full domain
-        result = util.distinctIntegers(HIGH - 1, HIGH);
+        result = RandomUtil.distinctIntegers(random, HIGH - 1, HIGH);
         Assert.assertEquals(result.length, HIGH - 1);
         Assert.assertEquals(new HashSet<>(Arrays.asList(box(result))).size(), HIGH - 1);
         Assert.assertTrue(Arrays.asList(array).containsAll(Arrays.asList(box(result))));
     }
 
     @Test
     public void testSampleUnique() {
-        Assert.assertEquals(util.sampleUnique(Collections.emptyList(), HIGH), Collections.emptyList());
+        Assert.assertEquals(RandomUtil.sampleUnique(random, Collections.emptyList(), HIGH), Collections.emptyList());
 
-        List<Integer> result = util.sampleUnique(list, HIGH);
+        List<Integer> result = RandomUtil.sampleUnique(random, list, HIGH);
         Assert.assertEquals(result.size(), HIGH);
         result.sort(Integer::compareTo);
         Assert.assertEquals(result, list);
 
         // Sample all but one from full domain
-        result = util.sampleUnique(list, HIGH - 1);
+        result = RandomUtil.sampleUnique(random, list, HIGH - 1);
         Assert.assertEquals(result.size(), HIGH - 1);
         Assert.assertEquals(new HashSet<>(result).size(), HIGH - 1);
         Assert.assertTrue(list.containsAll(result));
     }
 
     @Test
     public void testSampleList() {
-        Assert.assertEquals(util.sample(Collections.emptyList(), HIGH), Collections.emptyList());
+        Assert.assertEquals(RandomUtil.sample(random, Collections.emptyList(), HIGH), Collections.emptyList());
 
-        List<Integer> result = util.sample(list, HIGH);
+        List<Integer> result = RandomUtil.sample(random, list, HIGH);
         Assert.assertEquals(result.size(), HIGH);
         Assert.assertTrue(list.containsAll(result));
 
         // Sample double the size from full domain
-        result = util.sample(list, HIGH * 2);
+        result = RandomUtil.sample(random, list, HIGH * 2);
         Assert.assertEquals(result.size(), HIGH * 2);
         Assert.assertTrue(list.containsAll(result));
 

commons/util/src/test/java/net/automatalib/common/util/random/RandomUtilTest.java

@@ -122,6 +122,9 @@ public int getNodeId(Integer node) {
 
     @Override
     public Integer getNode(int id) {
+        if (id < 0 || id >= size) {
+            throw new IllegalArgumentException();
+        }
         return id;
     }