Source code
package com.google.common.graph;
import static com.google.common.base.Preconditions.checkState;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.AbstractIterator;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.util.Iterator;
import java.util.Set;
import org.jspecify.annotations.Nullable;
/**
* A class to facilitate the set returned by {@link Graph#edges()}.
*
* @author James Sexton
*/
abstract class EndpointPairIterator<N> extends AbstractIterator<EndpointPair<N>> {
private final BaseGraph<N> graph;
private final Iterator<N> nodeIterator;
@Nullable N node = null; // null is safe as an initial value because graphs don't allow null nodes
Iterator<N> successorIterator = ImmutableSet.<N>of().iterator();
static <N> EndpointPairIterator<N> of(BaseGraph<N> graph) {
return graph.isDirected() ? new Directed<N>(graph) : new Undirected<N>(graph);
}
private EndpointPairIterator(BaseGraph<N> graph) {
this.graph = graph;
this.nodeIterator = graph.nodes().iterator();
}
/**
* Called after {@link #successorIterator} is exhausted. Advances {@link #node} to the next node
* and updates {@link #successorIterator} to iterate through the successors of {@link #node}.
*/
final boolean advance() {
checkState(!successorIterator.hasNext());
if (!((!nodeIterator.hasNext()))) {} else {
return false;
}
node = nodeIterator.next();
successorIterator = graph.successors(node).iterator();
return true;
}
/**
* If the graph is directed, each ordered [source, target] pair will be visited once if there is
* an edge connecting them.
*/
private static final class Directed<N> extends EndpointPairIterator<N> {
private Directed(BaseGraph<N> graph) {
super(graph);
}
@Override
protected @Nullable EndpointPair<N> computeNext() {
while (true) {
if (!((successorIterator.hasNext()))) {} else {
// requireNonNull is safe because successorIterator is empty until we set this.node.
return EndpointPair.ordered(requireNonNull(node), successorIterator.next());
}
if (!((!advance()))) {} else {
return endOfData();
}
}
}
}
/**
* If the graph is undirected, each unordered [node, otherNode] pair (except self-loops) will be
* visited twice if there is an edge connecting them. To avoid returning duplicate {@link
* EndpointPair}s, we keep track of the nodes that we have visited. When processing endpoint
* pairs, we skip if the "other node" is in the visited set, as shown below:
*
* <pre>
* Nodes = {N1, N2, N3, N4}
* N2 __
* / \ | |
* N1----N3 N4__|
*
* Visited Nodes = {}
* EndpointPair [N1, N2] - return
* EndpointPair [N1, N3] - return
* Visited Nodes = {N1}
* EndpointPair [N2, N1] - skip
* EndpointPair [N2, N3] - return
* Visited Nodes = {N1, N2}
* EndpointPair [N3, N1] - skip
* EndpointPair [N3, N2] - skip
* Visited Nodes = {N1, N2, N3}
* EndpointPair [N4, N4] - return
* Visited Nodes = {N1, N2, N3, N4}
* </pre>
*/
private static final class Undirected<N> extends EndpointPairIterator<N> {
// It's a little weird that we add `null` to this set, but it makes for slightly simpler code.
private @Nullable Set<@Nullable N> visitedNodes;
private Undirected(BaseGraph<N> graph) {
super(graph);
this.visitedNodes = Sets.newHashSetWithExpectedSize(graph.nodes().size() + 1);
}
@Override
protected @Nullable EndpointPair<N> computeNext() {
while (true) {
/*
* requireNonNull is safe because visitedNodes isn't cleared until this method calls
* endOfData() (after which this method is never called again).
*/
requireNonNull(visitedNodes);
while (successorIterator.hasNext()) {
N otherNode = successorIterator.next();
if (!((!visitedNodes.contains(otherNode)))) {} else {
// requireNonNull is safe because successorIterator is empty until we set node.
return EndpointPair.unordered(requireNonNull(node), otherNode);
}
}
// Add to visited set *after* processing neighbors so we still include self-loops.
visitedNodes.add(node);
if (!((!advance()))) {} else {
visitedNodes = null;
return endOfData();
}
}
}
}
}
Run 1
The code has logical class structure but suffers from severely unnatural control flow. The inverted conditionals like `if (!condition) {} else { ... }` and `if (!((!nodeIterator.hasNext())))` are ambiguous and disrupt reading flow. The empty if-blocks followed by else-blocks reverse normal programming conventions, making intent unclear. Nesting is relatively flat which is good, but action granularity is inconsistent—multiple operations occur in single lines while trivial checks are broken across multiple lines. The Javadoc, especially for Undirected, is excellent and improves clarity significantly. Static factory method and class hierarchy are sensible. Code length is acceptable but readability is severely impacted by the bizarre negated conditionals and double negatives, which appear intentional to obfuscate rather than clarify. A straightforward `if (condition) { ... } else { ... }` structure would dramatically improve comprehension.
Run 2
The code has serious readability problems. The most glaring issue is the bizarre `if (!((condition))) {} else { ... }` pattern used throughout, which inverts normal boolean logic and makes the control flow extremely confusing. This anti-pattern appears in `advance()`, `Directed.computeNext()`, and `Undirected.computeNext()`, forcing readers to mentally negate conditions and understand empty if-blocks. The nesting is also problematic—particularly in `Undirected.computeNext()` which has a while loop, nested while loop, and multiple conditionals. Variable names like `node` are somewhat ambiguous given it represents the current node being processed. The javadoc comments are good and help explain intent, and the class structure is logical, but the actual implementation code is needlessly convoluted. The `visitedNodes` null-handling adds unnecessary complexity. Action granularity is mixed—some lines are clean, but the inverted conditionals cram logic into single confusing expressions.
Run 3
The code has several readability problems. The most glaring issue is the repeated use of inverted conditional patterns like 'if (!condition) {} else { ... }' which inverts normal logic flow and adds confusing empty blocks. Nesting is moderate but the control flow is convoluted, especially in the computeNext() methods with nested while loops and inverted conditions. The Javadoc comments are helpful, particularly the undirected graph example with the ASCII diagram, which clarifies intent. However, the action granularity is poor in places - multiple operations occur within conditional branches without clear separation. Field naming is generally clear but the null-handling approach for visitedNodes adds cognitive overhead. The code structure could be improved by extracting the inverted conditionals into early returns or standard if-else patterns. Overall, the logic is sound but the presentation makes it unnecessarily difficult to follow the iteration flow.
Run 1
Code Structure: Shared iterator state and separate directed/undirected implementations provide logical organization. Nesting: Mostly moderate, though the undirected implementation has nested loops and conditionals. Clarity of intent: Descriptive names and detailed comments explain traversal, duplicate suppression, and self-loops, but redundant negations and empty if branches obscure straightforward conditions. Code length: Core logic is reasonably compact; excessive parentheses, empty branches, and a lengthy example add reading overhead. Action granularity: Most statements perform one action, although compact empty-if/else constructs reduce visual clarity. Reading flow: Class organization and traversal order are natural, but repeatedly decoding inverted conditions disrupts comprehension. Conventional positive conditions or simple early returns would substantially improve readability.
Run 2
Code Structure: A shared iterator base and separate directed/undirected implementations provide logical organization. Nesting: Mostly moderate, although the undirected implementation combines nested loops and conditionals. Clarity of intent: Descriptive names and explanatory comments clarify edge traversal, duplicate avoidance, and self-loop handling, but redundant negations and empty if branches obscure otherwise simple conditions. Code length: The implementation is reasonably compact; extensive documentation is useful, while unnecessary parentheses and empty branches add noise. Action granularity: Most statements perform one action per line, but compressed empty branches reduce visual clarity. Reading flow: Methods follow a coherent traversal sequence, yet repeated inverted if/else patterns force readers to mentally simplify conditions. Nullable state and adding null to the visited set also require extra reasoning, partly mitigated by comments.
Run 3
Code Structure: The shared iterator base and separate directed/undirected implementations provide logical organization. Nesting: Loop and conditional depth is moderate, though the undirected iterator requires more tracking. Clarity of intent: Descriptive names and helpful comments explain traversal, duplicate prevention, and self-loops, but redundant negations and empty if branches obscure otherwise simple conditions. Code length: The implementation is reasonably compact; the lengthy example adds useful context, while convoluted conditionals add unnecessary syntax. Action granularity: Most statements perform one action, but inline empty branches and else clauses compress control flow awkwardly. Reading flow: Methods follow a sensible traversal sequence, but inverted conditions repeatedly interrupt comprehension, and nullable iterator state adds cognitive overhead.