Using guava for the cycle perception code - note some tests changed a…

…s the initial cycles are now always returned in lexicography order.

src/META-INF/core.libdepends

@@ -0,0 +1 @@
+guava-14.0.1.jar

src/META-INF/test-core.libdepends

@@ -1,3 +1,4 @@
 log4j*.jar
 vecmath*.jar
 jgrapht-0.6.0.jar
+guava-14.0.1.jar

src/main/org/openscience/cdk/graph/EssentialCycles.java

@@ -33,6 +33,7 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.openscience.cdk.graph.InitialCycles.Cycle;
 
 /**
@@ -96,10 +97,10 @@ public EssentialCycles(final int[][] graph) {
      * @param initial a molecule graph
      */
     EssentialCycles(final RelevantCycles relevant, final InitialCycles initial) {
-
-        this.initial = initial;
-        this.basis = new GreedyBasis(initial.numberOfCycles(),
-                                     initial.numberOfEdges());
+        checkNotNull(relevant, "No RelevantCycles provided");
+        this.initial = checkNotNull(initial, "No iInitialCycles provided");
+        this.basis   = new GreedyBasis(initial.numberOfCycles(),
+                                       initial.numberOfEdges());
         this.essential = new ArrayList<Cycle>();
 
         // for each cycle added to the basis, if it can be

src/main/org/openscience/cdk/graph/InitialCycles.java

@@ -23,18 +23,20 @@
  */
 package org.openscience.cdk.graph;
 
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.TreeMultimap;
+import com.google.common.primitives.Ints;
 import org.openscience.cdk.annotations.TestClass;
 import org.openscience.cdk.annotations.TestMethod;
 
-import java.util.ArrayList;
 import java.util.BitSet;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.Collection;
 
 import static java.util.Arrays.copyOf;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * Compute the set of initial cycles (<i>C'<sub>I</sub></i>) in a graph. The
@@ -56,15 +58,11 @@ final class InitialCycles {
     /** Vertex ordering. */
     private final int[] ordering;
 
-    // TODO: can be replaced with Multimap if guava is added
     /** Cycle prototypes indexed by their length. */
-    private final Map<Integer, List<Cycle>> cycles = new TreeMap<Integer, List<Cycle>>();
-    private int nCycles = 0;
+    private final Multimap<Integer, Cycle> cycles = TreeMultimap.create();
 
-    // TODO: can be replaced with BiMap if guava is added
     /** Index of edges in the graph */
-    private final Map<Edge, Integer> edgeToIndex;
-    private final List<Edge> edges;
+    private final BiMap<Edge, Integer> edges;
 
     /**
      * Initial array size for 'ordering()'. This method sorts vertices by degree
@@ -84,9 +82,7 @@ final class InitialCycles {
      * @throws NullPointerException the graphw as null
      */
     InitialCycles(final int[][] graph) {
-        if (graph == null)
-            throw new NullPointerException("no graph provided");
-        this.graph = graph;
+        this.graph = checkNotNull(graph, "no graph provided");
 
         // ordering ensures the number of initial cycles is polynomial
         this.ordering = ordering(graph);
@@ -95,15 +91,13 @@ final class InitialCycles {
         // - edge representation: binary vector indicates whether an edge
         //                        is present or
         // - path representation: sequential list vertices forming the cycle
-        edges = new ArrayList<Edge>(graph.length * 2);
-        edgeToIndex = new HashMap<Edge, Integer>(graph.length * 2);
+        edges = HashBiMap.create(graph.length);
         int n = graph.length;
         for (int v = 0; v < n; v++) {
             for (int w : graph[v]) {
                 if (w > v) {
                     Edge edge = new Edge(v, w);
-                    edgeToIndex.put(edge, edges.size());
-                    edges.add(edge);
+                    edges.put(edge, edges.size());
                 }
             }
         }
@@ -128,8 +122,8 @@ final class InitialCycles {
      */
     @TestMethod("lengths_K4,lengths_napthalene,lengths_anthracene," +
                         "lengths_bicyclo,lengths_cyclophane")
-    List<Integer> lengths() {
-        return new ArrayList<Integer>(cycles.keySet());
+    Iterable<Integer> lengths() {
+        return cycles.keySet();
     }
 
     /**
@@ -141,9 +135,8 @@ List<Integer> lengths() {
      * @see #lengths()
      */
     @TestMethod("cyclesOfLength_empty,cycles_K4")
-    List<Cycle> cyclesOfLength(int length) {
-        List<Cycle> cycles = this.cycles.get(length);
-        return cycles != null ? cycles : Collections.<Cycle>emptyList();
+    Collection<Cycle> cyclesOfLength(int length) {
+        return cycles.get(length);
     }
 
     /**
@@ -153,12 +146,8 @@ List<Cycle> cyclesOfLength(int length) {
      */
     @TestMethod("cycles_K4,cycles_napthalene,cycles_anthracene," +
                         "cycles_bicyclo,cycles_cyclophane")
-    List<Cycle> cycles() {
-        List<Cycle> tmp = new ArrayList<Cycle>();
-        for (List<Cycle> ps : cycles.values()) {
-            tmp.addAll(ps);
-        }
-        return tmp;
+    Collection<Cycle> cycles() {
+        return cycles.values();
     }
 
     /**
@@ -167,7 +156,7 @@ List<Cycle> cycles() {
      * @return number of cycles
      */
     @TestMethod("numberOfCycles_K4") int numberOfCycles() {
-        return nCycles;
+        return cycles.size();
     }
 
     /**
@@ -186,7 +175,7 @@ List<Cycle> cycles() {
      * @return the edge at the given index
      */
     @TestMethod("edge_K4") Edge edge(int i) {
-        return edges.get(i);
+        return edges.inverse().get(i);
     }
 
     /**
@@ -198,7 +187,7 @@ List<Cycle> cycles() {
      * @return the index of the edge
      */
     @TestMethod("indexOfEdge_K4") int indexOfEdge(final int u, final int v) {
-        return edgeToIndex.get(new Edge(u, v));
+        return edges.get(new Edge(u, v));
     }
 
     /**
@@ -341,14 +330,7 @@ else if (distToZ == distToY && ordering[z] < ordering[y]) {
      * @param cycle the cycle to add
      */
     private void add(Cycle cycle) {
-        int length = cycle.length();
-        List<Cycle> ps = cycles.get(length);
-        if (ps == null) {
-            ps = new ArrayList<Cycle>(2);
-            cycles.put(length, ps);
-        }
-        ps.add(cycle);
-        nCycles++;
+        cycles.put(cycle.length(), cycle);
     }
 
     /**
@@ -442,7 +424,7 @@ static int[] join(int[] pathToP, int y, int[] pathToQ) {
      * Abstract description of a cycle. Stores the path and computes the edge
      * vector representation.
      */
-    static abstract class Cycle {
+    static abstract class Cycle implements Comparable<Cycle> {
 
         private int[] path;
         ShortestPaths paths;
@@ -505,6 +487,11 @@ int[] path() {
         int length() {
             return path.length;
         }
+
+        @Override public int compareTo(Cycle that) {
+            return Ints.lexicographicalComparator().compare(this.path,
+                                                            that.path);
+        }
     }
 
     /**

src/main/org/openscience/cdk/graph/MinimumCycleBasis.java

@@ -26,6 +26,7 @@
 import org.openscience.cdk.annotations.TestClass;
 import org.openscience.cdk.annotations.TestMethod;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.openscience.cdk.graph.InitialCycles.Cycle;
 
 /**
@@ -98,8 +99,7 @@ public MinimumCycleBasis(final int[][] graph) {
     @TestMethod("noInitialCycles")
     MinimumCycleBasis(final InitialCycles initial) {
 
-        if (initial == null)
-            throw new NullPointerException("no InitialCycles provided");
+        checkNotNull(initial, "No InitialCycles provided");
 
         this.basis = new GreedyBasis(initial.numberOfCycles(),
                                      initial.numberOfEdges());

src/main/org/openscience/cdk/graph/RelevantCycles.java

@@ -28,8 +28,10 @@
 import org.openscience.cdk.annotations.TestMethod;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.openscience.cdk.graph.InitialCycles.Cycle;
 
 /**
@@ -114,8 +116,7 @@ public RelevantCycles(final int[][] graph) {
      */
     @TestMethod("noInitialCycles") RelevantCycles(final InitialCycles initial) {
 
-        if (initial == null)
-            throw new NullPointerException("no InitialCycles provided");
+        checkNotNull(initial, "No InitialCycles provided");
 
         this.basis = new GreedyBasis(initial.numberOfCycles(),
                                      initial.numberOfEdges());
@@ -133,7 +134,7 @@ public RelevantCycles(final int[][] graph) {
      * @param cycles cycles of a given length
      * @return cycles which were independent
      */
-    private List<Cycle> independent(final List<Cycle> cycles) {
+    private List<Cycle> independent(final Collection<Cycle> cycles) {
         final List<Cycle> independent = new ArrayList<Cycle>(cycles.size());
         for (final Cycle cycle : cycles) {
             if (basis.isIndependent(cycle))

src/test/org/openscience/cdk/graph/EssentialCyclesTest.java

@@ -59,8 +59,8 @@ public class EssentialCyclesTest {
         EssentialCycles essential = new EssentialCycles(anthracene);
         int[][] paths = essential.paths();
         int[][] expected = new int[][]{{5, 0, 1, 2, 3, 4},
-                                       {9, 8, 10, 11, 12, 13},
-                                       {9, 6, 5, 4, 7, 8}};
+                                       {9, 6, 5, 4, 7, 8},
+                                       {9, 8, 10, 11, 12, 13}};
         assertThat(paths, is(expected));
     }
 

src/test/org/openscience/cdk/graph/InitialCyclesTest.java

@@ -23,9 +23,11 @@
  */
 package org.openscience.cdk.graph;
 
+import com.google.common.collect.Lists;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.hasItem;
@@ -41,7 +43,7 @@
 public class InitialCyclesTest {
 
     @Test public void lengths_empty() {
-        assertTrue(new InitialCycles(new int[0][0]).lengths().isEmpty());
+        assertFalse(new InitialCycles(new int[0][0]).lengths().iterator().hasNext());
     }
 
     @Test public void cyclesOfLength_empty() {
@@ -56,7 +58,7 @@ public class InitialCyclesTest {
     }
 
     @Test public void lengths_K1() {
-        assertTrue(new InitialCycles(k1()).lengths().isEmpty());
+        assertFalse(new InitialCycles(k1()).lengths().iterator().hasNext());
     }
 
     @Test public void cycles_K1() {
@@ -118,40 +120,37 @@ public class InitialCyclesTest {
     }
 
     @Test public void lengths_napthalene() throws IOException {
-        assertThat(new InitialCycles(napthalene()).lengths().size(), is(1));
         assertThat(new InitialCycles(napthalene()).lengths(), hasItem(6));
     }
 
     @Test public void cycles_napthalene() throws IOException {
         InitialCycles initial = new InitialCycles(napthalene());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles =  Lists.newArrayList(initial.cycles());
         assertThat(cycles.size(), is(2));
         assertThat(cycles.get(0).path(), is(new int[]{5, 0, 1, 2, 3, 4}));
         assertThat(cycles.get(1).path(), is(new int[]{5, 4, 7, 8, 9, 6}));
     }
 
     @Test public void lengths_anthracene() throws IOException {
-        assertThat(new InitialCycles(anthracene()).lengths().size(), is(1));
         assertThat(new InitialCycles(anthracene()).lengths(), hasItem(6));
     }
 
     @Test public void cycles_anthracene() throws IOException {
         InitialCycles initial = new InitialCycles(anthracene());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles = Lists.newArrayList(initial.cycles());
         assertThat(cycles.size(), is(3));
         assertThat(cycles.get(0).path(), is(new int[]{5, 0, 1, 2, 3, 4}));
-        assertThat(cycles.get(1).path(), is(new int[]{9, 8, 10, 11, 12, 13}));
-        assertThat(cycles.get(2).path(), is(new int[]{9, 6, 5, 4, 7, 8}));
+        assertThat(cycles.get(1).path(), is(new int[]{9, 6, 5, 4, 7, 8}));
+        assertThat(cycles.get(2).path(), is(new int[]{9, 8, 10, 11, 12, 13}));
     }
 
     @Test public void lengths_bicyclo() throws IOException {
-        assertThat(new InitialCycles(bicyclo()).lengths().size(), is(1));
         assertThat(new InitialCycles(bicyclo()).lengths(), hasItem(6));
     }
 
     @Test public void cycles_bicyclo() throws IOException {
         InitialCycles initial = new InitialCycles(bicyclo());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles =  Lists.newArrayList(initial.cycles());
         assertThat(cycles.size(), is(3));
         assertThat(cycles.get(0).path(), is(new int[]{5, 0, 1, 2, 3, 4}));
         assertThat(cycles.get(1).path(), is(new int[]{5, 0, 1, 2, 7, 6}));
@@ -160,18 +159,19 @@ public class InitialCyclesTest {
 
     @Test public void lengths_cyclophane() throws IOException {
         InitialCycles initial = new InitialCycles(cyclophane_odd());
-        assertThat(initial.lengths().size(), is(2));
         assertThat(initial.lengths(),
                    hasItems(6, 9));
-        assertThat(initial.lengths().get(0),
+        Iterator<Integer> it = initial.lengths().iterator();
+        assertThat(it.next(),
                    is(6));
-        assertThat(initial.lengths().get(1),
+        assertThat(it.next(),
                    is(9));
+        assertFalse(it.hasNext());
     }
 
     @Test public void cycles_cyclophane() throws IOException {
         InitialCycles initial = new InitialCycles(cyclophane_odd());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles =  Lists.newArrayList(initial.cycles());
         assertThat(cycles.size(), is(2));
         assertThat(cycles.get(0).path(), is(new int[]{3, 2, 1, 0, 5, 4}));
         assertThat(cycles.get(1)
@@ -180,7 +180,7 @@ public class InitialCyclesTest {
 
     @Test public void cycles_family_odd() {
         InitialCycles initial = new InitialCycles(cyclophane_odd());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles =  Lists.newArrayList(initial.cycles());
         assertThat(cycles.get(1)
                          .path(), is(new int[]{3, 2, 1, 0, 10, 9, 8, 7, 6}));
         int[][] family = cycles.get(1).family();
@@ -191,7 +191,7 @@ public class InitialCyclesTest {
 
     @Test public void cycles_family_even() {
         InitialCycles initial = new InitialCycles(cyclophane_even());
-        List<InitialCycles.Cycle> cycles = initial.cycles();
+        List<InitialCycles.Cycle> cycles =  Lists.newArrayList(initial.cycles());
         assertThat(cycles.get(1).path(),
                    is(new int[]{3, 6, 7, 8, 9, 10, 11, 0, 1, 2}));
         int[][] family = cycles.get(1).family();

src/test/org/openscience/cdk/graph/MinimumCycleBasisTest.java

@@ -74,8 +74,8 @@ public void noInitialCycles() {
         int[][] paths = mcb.paths();
         assertThat(paths.length, is(3));
         int[][] expected = new int[][]{{5, 0, 1, 2, 3, 4},
-                                       {9, 8, 10, 11, 12, 13},
-                                       {9, 6, 5, 4, 7, 8}};
+                                       {9, 6, 5, 4, 7, 8},
+                                       {9, 8, 10, 11, 12, 13}};
         assertThat(paths, is(expected));
     }