Add ValueGraph.edgeValues(). Remove ValueGraph.edgeValueOrDefault(nod…

…e, node).

There's a few nice things we get out of this. An easy way to address all the values in a ValueGraph as a single collection (edgeValues().values()). An easy way to get an edge value if you're dealing with "EndpointPair"s. And if you're on Java 8+, Map has getOrDefault() plus some other nifty methods.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=131959724

guava-tests/test/com/google/common/graph/GraphsTest.java

@@ -278,10 +278,11 @@ public void transpose_directedValueGraph() {
     assertThat(transpose(transpose)).isSameAs(directedGraph);
     AbstractGraphTest.validateGraph(transpose);
 
-    assertThat(transpose.edgeValueOrDefault(N1, N2, null)).isNull();
+    EndpointPair<Integer> pair12 = EndpointPair.ordered(N1, N2);
+    assertThat(transpose.edgeValues().get(pair12)).isNull();
     directedGraph.putEdgeValue(N2, N1, E21);
     // View should be updated.
-    assertThat(transpose.edgeValueOrDefault(N1, N2, null)).isEqualTo(E21);
+    assertThat(transpose.edgeValues().get(pair12)).isEqualTo(E21);
     AbstractGraphTest.validateGraph(transpose);
   }
 

guava-tests/test/com/google/common/graph/ValueGraphTest.java

@@ -19,6 +19,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
+import java.util.Map;
 import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -37,12 +38,17 @@ public void validateGraphState() {
     assertThat(Graphs.equivalent(graph, Graphs.copyOf(graph))).isTrue();
     assertThat(Graphs.equivalent(graph, ImmutableValueGraph.copyOf(graph))).isTrue();
 
+    assertThat(graph.edgeValues().keySet()).isEqualTo(graph.edges());
+
     for (Integer node : graph.nodes()) {
       for (Integer otherNode : graph.nodes()) {
+        EndpointPair<Integer> endpointPair = EndpointPair.of(graph, node, otherNode);
         if (graph.successors(node).contains(otherNode)) {
-          assertThat(graph.edgeValue(node, otherNode)).isNotNull();
+          String value = graph.edgeValue(node, otherNode);
+          assertThat(value).isNotNull();
+          assertThat(value).isEqualTo(graph.edgeValues().get(endpointPair));
         } else {
-          assertThat(graph.edgeValueOrDefault(node, otherNode, null)).isNull();
+          assertThat(graph.edgeValues()).doesNotContainKey(endpointPair);
         }
       }
     }
@@ -167,23 +173,32 @@ public void edgeValue_nodeNotPresent() {
   }
 
   @Test
-  public void edgeValueOrDefault() {
+  public void edgeValues() {
     graph = ValueGraphBuilder.directed().build();
+    Map<EndpointPair<Integer>, String> edgeValues = graph.edgeValues();
+    EndpointPair<Integer> pair12 = EndpointPair.ordered(1, 2);
+    EndpointPair<Integer> pair21 = EndpointPair.ordered(2, 1);
 
     graph.addNode(1);
     graph.addNode(2);
-    assertThat(graph.edgeValueOrDefault(1, 2, "default")).isEqualTo("default");
-    assertThat(graph.edgeValueOrDefault(2, 1, "default")).isEqualTo("default");
+    assertThat(edgeValues).doesNotContainKey(pair12);
+    assertThat(edgeValues).doesNotContainKey(pair21);
 
     graph.putEdgeValue(1, 2, "valueA");
     graph.putEdgeValue(2, 1, "valueB");
-    assertThat(graph.edgeValueOrDefault(1, 2, "default")).isEqualTo("valueA");
-    assertThat(graph.edgeValueOrDefault(2, 1, "default")).isEqualTo("valueB");
+    assertThat(edgeValues.get(pair12)).isEqualTo("valueA");
+    assertThat(edgeValues.get(pair21)).isEqualTo("valueB");
 
     graph.removeEdge(1, 2);
     graph.putEdgeValue(2, 1, "valueC");
-    assertThat(graph.edgeValueOrDefault(1, 2, "default")).isEqualTo("default");
-    assertThat(graph.edgeValueOrDefault(2, 1, "default")).isEqualTo("valueC");
+    assertThat(edgeValues).doesNotContainKey(pair12);
+    assertThat(edgeValues.get(pair21)).isEqualTo("valueC");
+
+    try {
+      edgeValues.put(pair12, "valueA");
+      fail("Map returned by edgeValues() should be unmodifiable");
+    } catch (UnsupportedOperationException expected) {
+    }
   }
 
   @Test

guava/src/com/google/common/graph/AbstractValueGraph.java

@@ -36,6 +36,21 @@
 public abstract class AbstractValueGraph<N, V>
     extends AbstractGraph<N> implements ValueGraph<N, V> {
 
+  @Override
+  public Map<EndpointPair<N>, V> edgeValues() {
+    return edgeValues(this);
+  }
+
+  static <N, V> Map<EndpointPair<N>, V> edgeValues(final ValueGraph<N, V> graph) {
+    Function<EndpointPair<N>, V> edgeToValueFn = new Function<EndpointPair<N>, V>() {
+      @Override
+      public V apply(EndpointPair<N> edge) {
+        return graph.edgeValue(edge.nodeU(), edge.nodeV());
+      }
+    };
+    return Maps.asMap(graph.edges(), edgeToValueFn);
+  }
+
   /**
    * Returns a string representation of this graph.
    */
@@ -50,16 +65,6 @@ static String toString(ValueGraph<?, ?> graph) {
     return String.format(GRAPH_STRING_FORMAT,
         propertiesString,
         graph.nodes(),
-        edgeValueMap(graph));
-  }
-
-  private static <N, V> Map<EndpointPair<N>, V> edgeValueMap(final ValueGraph<N, V> graph) {
-    Function<EndpointPair<N>, V> edgeToValueFn = new Function<EndpointPair<N>, V>() {
-      @Override
-      public V apply(EndpointPair<N> edge) {
-        return graph.edgeValue(edge.nodeU(), edge.nodeV());
-      }
-    };
-    return Maps.asMap(graph.edges(), edgeToValueFn);
+        graph.edgeValues());
   }
 }

guava/src/com/google/common/graph/ConfigurableValueGraph.java

@@ -126,17 +126,10 @@ public Set<N> successors(Object node) {
 
   @Override
   public V edgeValue(Object nodeU, Object nodeV) {
-    V value = edgeValueOrDefault(nodeU, nodeV, null);
-    checkArgument(value != null, EDGE_CONNECTING_NOT_IN_GRAPH, nodeU, nodeV);
-    return value;
-  }
-
-  @Override
-  public V edgeValueOrDefault(Object nodeU, Object nodeV, @Nullable V defaultValue) {
     V value = checkedConnections(nodeU).value(nodeV);
     if (value == null) {
       checkArgument(containsNode(nodeV), NODE_NOT_IN_GRAPH, nodeV);
-      return defaultValue;
+      throw new IllegalArgumentException(String.format(EDGE_CONNECTING_NOT_IN_GRAPH, nodeU, nodeV));
     }
     return value;
   }

guava/src/com/google/common/graph/Graph.java

@@ -74,12 +74,12 @@
  * should prefer the non-mutating {@link Graph} interface.
  *
  * <p>We provide an efficient implementation of this interface via {@link GraphBuilder}. When using
- * the implementation provided, all {@link Set}-returning methods provide live, unmodifiable views
- * of the graph. In other words, you cannot add an element to the {@link Set}, but if an element is
- * added to the {@link Graph} that would affect the result of that set, it will be updated
+ * the implementation provided, all collection-returning methods provide live, unmodifiable views of
+ * the graph. In other words, you cannot add an element to the collection, but if an element is
+ * added to the {@link Graph} that would affect the result of that collection, it will be updated
  * automatically. This also means that you cannot modify a {@link Graph} in a way that would affect
- * a {#link Set} while iterating over that set. For example, you cannot remove the nodes from a
- * {@link Graph} while iterating over {@link #nodes} (unless you first make a copy of the nodes),
+ * a collection while iterating over that collection. For example, you cannot remove the nodes from
+ * a {@link Graph} while iterating over {@link #nodes} (unless you first make a copy of the nodes),
  * just as you could not remove the keys from a {@link Map} while iterating over its {@link
  * Map#keySet()}. This will either throw a {@link ConcurrentModificationException} or risk undefined
  * behavior.

guava/src/com/google/common/graph/Graphs.java

@@ -267,21 +267,9 @@ public static boolean equivalent(
       return false;
     }
 
-    // TODO(b/31166974): If we add a Map of edgeValues, we can check against that for equality.
-    if (graphA.isDirected() != graphB.isDirected()
-        || !graphA.nodes().equals(graphB.nodes())
-        || !graphA.edges().equals(graphB.edges())) {
-      return false;
-    }
-
-    for (EndpointPair<?> edge : graphA.edges()) {
-      if (!graphA.edgeValue(edge.nodeU(), edge.nodeV()).equals(
-          graphB.edgeValue(edge.nodeU(), edge.nodeV()))) {
-        return false;
-      }
-    }
-
-    return true;
+    return graphA.isDirected() == graphB.isDirected()
+        && graphA.nodes().equals(graphB.nodes())
+        && graphA.edgeValues().equals(graphB.edgeValues());
   }
 
   /**
@@ -471,10 +459,7 @@ public V edgeValue(Object nodeU, Object nodeV) {
       return graph.edgeValue(nodeV, nodeU); // transpose
     }
 
-    @Override
-    public V edgeValueOrDefault(Object nodeU, Object nodeV, V defaultValue) {
-      return graph.edgeValueOrDefault(nodeV, nodeU, defaultValue); // transpose
-    }
+    // Defer to AbstractValueGraph for edgeValues() implementation based on edgeValue().
   }
 
   /**

guava/src/com/google/common/graph/ImmutableValueGraph.java

@@ -26,7 +26,7 @@
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
-import javax.annotation.Nullable;
+import java.util.Map;
 
 /**
  * A {@link ValueGraph} whose elements and structural relationships will never change. Instances of
@@ -97,21 +97,19 @@ public V apply(N successorNode) {
 
   @Override
   public V edgeValue(Object nodeU, Object nodeV) {
-    V value = edgeValueOrDefault(nodeU, nodeV, null);
-    checkArgument(value != null, EDGE_CONNECTING_NOT_IN_GRAPH, nodeU, nodeV);
-    return value;
-  }
-
-  @Override
-  public V edgeValueOrDefault(Object nodeU, Object nodeV, @Nullable V defaultValue) {
     V value = backingGraph.checkedConnections(nodeU).value(nodeV);
     if (value == null) {
       checkArgument(backingGraph.containsNode(nodeV), NODE_NOT_IN_GRAPH, nodeV);
-      return defaultValue;
+      throw new IllegalArgumentException(String.format(EDGE_CONNECTING_NOT_IN_GRAPH, nodeU, nodeV));
     }
     return value;
   }
 
+  @Override
+  public Map<EndpointPair<N>, V> edgeValues() {
+    return AbstractValueGraph.edgeValues(this);
+  }
+
   @Override
   public String toString() {
     return AbstractValueGraph.toString(this);

guava/src/com/google/common/graph/Network.java

@@ -66,15 +66,15 @@
  * should prefer the non-mutating {@link Network} interface.
  *
  * <p>We provide an efficient implementation of this interface via {@link NetworkBuilder}. When
- * using the implementation provided, all {@link Set}-returning methods provide live, unmodifiable
- * views of the network. In other words, you cannot add an element to the {@link Set}, but if an
- * element is added to the {@link Network} that would affect the result of that set, it will be
- * updated automatically. This also means that you cannot modify a {@link Network} in a way that
- * would affect a {#link Set} while iterating over that set. For example, you cannot remove the
- * nodes from a {@link Network} while iterating over {@link #nodes} (unless you first make a copy of
- * the nodes), just as you could not remove the keys from a {@link Map} while iterating over its
- * {@link Map#keySet()}. This will either throw a {@link ConcurrentModificationException} or risk
- * undefined behavior.
+ * using the implementation provided, all collection-returning methods provide live, unmodifiable
+ * views of the network. In other words, you cannot add an element to the collection, but if an
+ * element is added to the {@link Network} that would affect the result of that collection, it will
+ * be updated automatically. This also means that you cannot modify a {@link Network} in a way that
+ * would affect a collection while iterating over that collection. For example, you cannot remove
+ * the nodes from a {@link Network} while iterating over {@link #nodes} (unless you first make a
+ * copy of the nodes), just as you could not remove the keys from a {@link Map} while iterating over
+ * its {@link Map#keySet()}. This will either throw a {@link ConcurrentModificationException} or
+ * risk undefined behavior.
  *
  * <p>Example of use:
  *

guava/src/com/google/common/graph/ValueGraph.java

@@ -19,7 +19,6 @@
 import com.google.common.annotations.Beta;
 import java.util.ConcurrentModificationException;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -74,15 +73,15 @@
  * should prefer the non-mutating {@link ValueGraph} interface.
  *
  * <p>We provide an efficient implementation of this interface via {@link ValueGraphBuilder}. When
- * using the implementation provided, all {@link Set}-returning methods provide live, unmodifiable
- * views of the graph. In other words, you cannot add an element to the {@link Set}, but if an
- * element is added to the {@link ValueGraph} that would affect the result of that set, it will be
- * updated automatically. This also means that you cannot modify a {@link ValueGraph} in a way that
- * would affect a {#link Set} while iterating over that set. For example, you cannot remove the
- * nodes from a {@link ValueGraph} while iterating over {@link #nodes} (unless you first make a copy
- * of the nodes), just as you could not remove the keys from a {@link Map} while iterating over its
- * {@link Map#keySet()}. This will either throw a {@link ConcurrentModificationException} or risk
- * undefined behavior.
+ * using the implementation provided, all collection-returning methods provide live, unmodifiable
+ * views of the graph. In other words, you cannot add an element to the collection, but if an
+ * element is added to the {@link ValueGraph} that would affect the result of that collection, it
+ * will be updated automatically. This also means that you cannot modify a {@link ValueGraph} in a
+ * way that would affect a collection while iterating over that collection. For example, you cannot
+ * remove the nodes from a {@link ValueGraph} while iterating over {@link #nodes} (unless you first
+ * make a copy of the nodes), just as you could not remove the keys from a {@link Map} while
+ * iterating over its {@link Map#keySet()}. This will either throw a
+ * {@link ConcurrentModificationException} or risk undefined behavior.
  *
  * <p>Example of use:
  *
@@ -120,16 +119,10 @@ public interface ValueGraph<N, V> extends Graph<N> {
   V edgeValue(Object nodeU, Object nodeV);
 
   /**
-   * If there is an edge connecting {@code nodeU} to {@code nodeV}, returns the non-null value
-   * associated with that edge; otherwise, returns {@code defaultValue}.
-   *
-   * <p>In an undirected graph, this is equal to {@code edgeValueOrDefault(nodeV, nodeU,
-   * defaultValue)}.
-   *
-   * @throws IllegalArgumentException if {@code nodeU} or {@code nodeV} is not an element of this
-   *     graph
+   * Returns a {@link Map} of all {@link #edges() edges} mapped to their associated {@link
+   * #edgeValue(Object, Object) value}.
    */
-  V edgeValueOrDefault(Object nodeU, Object nodeV, @Nullable V defaultValue);
+  Map<EndpointPair<N>, V> edgeValues();
 
   //
   // ValueGraph identity