Skip to content
This repository was archived by the owner on Jun 18, 2026. It is now read-only.

Commit 0876572

Browse files
fix(Edge): make equals/hashCode order-sensitive for directed-graph safety
The previous Edge.equals/hashCode (commit 11acb09) treated (v1, v2) and (v2, v1) as the same edge to model undirected semantics. That made it impossible to add both directions of an edge to a JUNG DirectedSparseGraph: JUNG's addEdge calls containsEdge(e) which delegates to Edge.equals, sees the reverse as already present, and throws IllegalArgumentException ("edge already exists with endpoints <A, B> and cannot be added with endpoints <B, A>"). This caused 17 pre-existing test failures across the directed-graph analyzers (CycleAnalyzer, StronglyConnectedComponentsAnalyzer, TopologicalSortAnalyzer, GraphTemporalDynamicsEngine). Changes - Edge.equals: order-sensitive on (vertex1, vertex2), still checks type and weight, still ignores label. - Edge.hashCode: consistent with the new equals (order-sensitive). - New Edge.equalsUndirected(Edge): preserves the legacy symmetric comparison for callers that want unordered-pair semantics. Use it explicitly when modeling undirected relationships on top of the standard equals contract. - Class javadoc updated to reflect ordered-pair identity. Tests - 15 new EdgeTest cases covering reflexivity, label-exclusion, order-sensitivity (regression guard), null/foreign-type handling, hashCode stability, equalsUndirected behavior, and a direct DirectedSparseGraph integration test that both directions can coexist. - GraphTemporalDynamicsEngineTest.createGraph: dedupe undirected edges via findEdge so test fixtures stay simple graphs (the old helper relied on the bug to silently swallow duplicates). - testDegreeEntropyIncreases: rebuilt the third snapshot with distinct edges (the previous version contained duplicate {A,D} pairs that collapsed under simple-graph semantics). Net effect on mvn test: 18 fewer failing tests (54 -> 36) with no new regressions; build still has 29F/8E of unrelated pre-existing failures (force-directed layout stack overflow, graph-labeling AIOOBE, shortest-path zero-weight handling, etc.).
1 parent e4b5631 commit 0876572

3 files changed

Lines changed: 201 additions & 19 deletions

File tree

Gvisual/src/gvisual/Edge.java

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@
1818
* {@code sg} (study group). See {@link EdgeType} for the full
1919
* palette and metadata.</li>
2020
* <li>Two endpoint identifiers ({@link #getVertex1()} and
21-
* {@link #getVertex2()}). The edge is treated as <b>undirected</b>:
22-
* {@code (v1, v2)} equals {@code (v2, v1)} for {@link #equals(Object)}
23-
* and {@link #hashCode()} purposes.</li>
21+
* {@link #getVertex2()}). The endpoint pair is treated as
22+
* <b>ordered</b> for {@link #equals(Object)} / {@link #hashCode()}
23+
* so the class can be used safely in directed graphs (where
24+
* {@code A->B} and {@code B->A} are distinct edges). For
25+
* undirected/symmetric comparisons, use
26+
* {@link #equalsUndirected(Edge)}.</li>
2427
* <li>A scalar {@link #getWeight() weight} (typically interaction
2528
* intensity — frequency × duration).</li>
2629
* <li>An optional human-readable {@link #getLabel() label}.</li>
@@ -215,8 +218,21 @@ public boolean isActiveDuring(long start, long end)
215218
}
216219

217220
/**
218-
* Two Edges are equal if they connect the same vertices (in either order),
219-
* have the same type, and the same weight.
221+
* Two Edges are equal if they have the same endpoints <em>in the same
222+
* order</em> ({@code vertex1} and {@code vertex2}), the same type, and
223+
* the same weight.
224+
*
225+
* <p><b>History:</b> previously this method treated {@code (v1, v2)}
226+
* and {@code (v2, v1)} as equal in an attempt to model undirected
227+
* semantics. That made it impossible to put an {@code Edge} into a
228+
* {@code DirectedSparseGraph} alongside its reverse: JUNG's
229+
* {@code containsEdge(e)} (which calls {@code Edge#equals}) reported
230+
* the reverse edge as already present and {@code addEdge} threw
231+
* {@code IllegalArgumentException}. The directed-graph use-cases
232+
* (strongly connected components, topological sort, cycle analysis,
233+
* temporal dynamics, etc.) are now first-class in the codebase, so
234+
* {@code equals} is order-sensitive. Callers that want the legacy
235+
* undirected comparison should use {@link #equalsUndirected(Edge)}.
220236
*/
221237
@Override
222238
public boolean equals(Object obj)
@@ -226,7 +242,26 @@ public boolean equals(Object obj)
226242
Edge other = (Edge) obj;
227243
if (Float.compare(weight, other.weight) != 0) return false;
228244
if (!java.util.Objects.equals(edgeType, other.edgeType)) return false;
229-
// Undirected: (v1,v2) == (v2,v1)
245+
return java.util.Objects.equals(vertex1, other.vertex1)
246+
&& java.util.Objects.equals(vertex2, other.vertex2);
247+
}
248+
249+
/**
250+
* Returns {@code true} if this edge connects the same pair of
251+
* vertices as {@code other} in <em>either</em> order, with the same
252+
* type and weight. Useful when modeling undirected relationships on
253+
* top of the standard {@link #equals(Object)} contract.
254+
*
255+
* @param other the edge to compare against; may be {@code null}
256+
* @return true if both edges share an unordered endpoint pair, type,
257+
* and weight
258+
*/
259+
public boolean equalsUndirected(Edge other)
260+
{
261+
if (this == other) return true;
262+
if (other == null) return false;
263+
if (Float.compare(weight, other.weight) != 0) return false;
264+
if (!java.util.Objects.equals(edgeType, other.edgeType)) return false;
230265
boolean sameOrder = java.util.Objects.equals(vertex1, other.vertex1)
231266
&& java.util.Objects.equals(vertex2, other.vertex2);
232267
boolean reverseOrder = java.util.Objects.equals(vertex1, other.vertex2)
@@ -235,16 +270,18 @@ public boolean equals(Object obj)
235270
}
236271

237272
/**
238-
* Hash code consistent with {@link #equals}: order-independent on vertices.
273+
* Hash code consistent with {@link #equals(Object)}: order-sensitive
274+
* on the endpoint pair.
239275
*/
240276
@Override
241277
public int hashCode()
242278
{
243-
// Use addition so vertex order doesn't matter
244-
int vertexHash = (vertex1 == null ? 0 : vertex1.hashCode())
245-
+ (vertex2 == null ? 0 : vertex2.hashCode());
246-
return 31 * (31 * vertexHash + (edgeType == null ? 0 : edgeType.hashCode()))
247-
+ Float.floatToIntBits(weight);
279+
int h = 1;
280+
h = 31 * h + (vertex1 == null ? 0 : vertex1.hashCode());
281+
h = 31 * h + (vertex2 == null ? 0 : vertex2.hashCode());
282+
h = 31 * h + (edgeType == null ? 0 : edgeType.hashCode());
283+
h = 31 * h + Float.floatToIntBits(weight);
284+
return h;
248285
}
249286

250287
/**

Gvisual/test/gvisual/EdgeTest.java

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,149 @@ public void testVerticesWithSpecialCharacters() {
143143

144144
@Test
145145
public void testSelfLoop() {
146-
// Edge class doesn't prevent self-loops verify it stores them
146+
// Edge class doesn't prevent self-loops - verify it stores them
147147
Edge e = new Edge("f", "X", "X");
148148
assertEquals(e.getVertex1(), e.getVertex2());
149149
}
150+
151+
// --- equals / hashCode contract (order-sensitive, directed-safe) ---
152+
153+
@Test
154+
public void testEqualsReflexive() {
155+
Edge e = new Edge("f", "A", "B");
156+
e.setWeight(1.5f);
157+
assertEquals(e, e);
158+
}
159+
160+
@Test
161+
public void testEqualsSameFieldsAreEqual() {
162+
Edge a = new Edge("f", "A", "B");
163+
a.setWeight(2.0f);
164+
Edge b = new Edge("f", "A", "B");
165+
b.setWeight(2.0f);
166+
assertEquals(a, b);
167+
assertEquals(a.hashCode(), b.hashCode());
168+
}
169+
170+
@Test
171+
public void testEqualsIgnoresLabel() {
172+
// Labels are display-only and excluded from identity.
173+
Edge a = new Edge("f", "A", "B");
174+
a.setLabel("close");
175+
Edge b = new Edge("f", "A", "B");
176+
b.setLabel("acquaintance");
177+
assertEquals(a, b);
178+
assertEquals(a.hashCode(), b.hashCode());
179+
}
180+
181+
@Test
182+
public void testEqualsIsOrderSensitive() {
183+
// Regression: previously equals treated (A,B) == (B,A), which made
184+
// it impossible to store both directions of an edge in a
185+
// DirectedSparseGraph (JUNG's addEdge would reject the reverse).
186+
Edge ab = new Edge("link", "A", "B");
187+
Edge ba = new Edge("link", "B", "A");
188+
assertNotEquals(ab, ba);
189+
// Hash codes should also differ for the swapped-vertex case so
190+
// hash-based containers don't collide spuriously.
191+
assertNotEquals(ab.hashCode(), ba.hashCode());
192+
}
193+
194+
@Test
195+
public void testEqualsDifferentTypeNotEqual() {
196+
Edge a = new Edge("f", "A", "B");
197+
Edge b = new Edge("c", "A", "B");
198+
assertNotEquals(a, b);
199+
}
200+
201+
@Test
202+
public void testEqualsDifferentWeightNotEqual() {
203+
Edge a = new Edge("f", "A", "B");
204+
a.setWeight(1.0f);
205+
Edge b = new Edge("f", "A", "B");
206+
b.setWeight(2.0f);
207+
assertNotEquals(a, b);
208+
}
209+
210+
@Test
211+
public void testEqualsAgainstNullAndOtherType() {
212+
Edge a = new Edge("f", "A", "B");
213+
assertNotEquals(a, null);
214+
assertNotEquals(a, "not-an-edge");
215+
}
216+
217+
@Test
218+
public void testHashCodeStableAcrossInvocations() {
219+
Edge e = new Edge("f", "A", "B");
220+
e.setWeight(3.25f);
221+
int h1 = e.hashCode();
222+
int h2 = e.hashCode();
223+
int h3 = e.hashCode();
224+
assertEquals(h1, h2);
225+
assertEquals(h2, h3);
226+
}
227+
228+
@Test
229+
public void testEqualsHandlesNullEndpoints() {
230+
Edge a = new Edge();
231+
Edge b = new Edge();
232+
// Two default-constructed edges have identical (all-null/zero) state.
233+
assertEquals(a, b);
234+
assertEquals(a.hashCode(), b.hashCode());
235+
}
236+
237+
// --- equalsUndirected (legacy symmetric comparison) ---
238+
239+
@Test
240+
public void testEqualsUndirectedTreatsReverseAsEqual() {
241+
Edge ab = new Edge("f", "A", "B");
242+
Edge ba = new Edge("f", "B", "A");
243+
assertTrue(ab.equalsUndirected(ba));
244+
assertTrue(ba.equalsUndirected(ab));
245+
}
246+
247+
@Test
248+
public void testEqualsUndirectedStillChecksTypeAndWeight() {
249+
Edge ab = new Edge("f", "A", "B");
250+
ab.setWeight(1.0f);
251+
Edge baDiffType = new Edge("c", "B", "A");
252+
baDiffType.setWeight(1.0f);
253+
Edge baDiffWeight = new Edge("f", "B", "A");
254+
baDiffWeight.setWeight(2.0f);
255+
assertFalse(ab.equalsUndirected(baDiffType));
256+
assertFalse(ab.equalsUndirected(baDiffWeight));
257+
}
258+
259+
@Test
260+
public void testEqualsUndirectedHandlesNull() {
261+
Edge ab = new Edge("f", "A", "B");
262+
assertFalse(ab.equalsUndirected(null));
263+
}
264+
265+
@Test
266+
public void testEqualsUndirectedReflexive() {
267+
Edge ab = new Edge("f", "A", "B");
268+
assertTrue(ab.equalsUndirected(ab));
269+
}
270+
271+
// --- Directed-graph integration (the bug this commit fixes) ---
272+
273+
@Test
274+
public void testDirectedGraphAcceptsBothDirections() {
275+
// Regression test for the equals/hashCode bug: with order-insensitive
276+
// equals, this addEdge throws IllegalArgumentException because
277+
// JUNG considers the reverse edge already present.
278+
edu.uci.ics.jung.graph.DirectedSparseGraph<String, Edge> g =
279+
new edu.uci.ics.jung.graph.DirectedSparseGraph<>();
280+
g.addVertex("A");
281+
g.addVertex("B");
282+
Edge ab = new Edge("link", "A", "B");
283+
ab.setLabel("e1");
284+
Edge ba = new Edge("link", "B", "A");
285+
ba.setLabel("e2");
286+
assertTrue("forward edge should be added", g.addEdge(ab, "A", "B"));
287+
assertTrue("reverse edge should be added in a directed graph",
288+
g.addEdge(ba, "B", "A"));
289+
assertEquals(2, g.getEdgeCount());
290+
}
150291
}

Gvisual/test/gvisual/GraphTemporalDynamicsEngineTest.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ public void setUp() {
2626
private Graph<String, Edge> createGraph(String[][] edges) {
2727
Graph<String, Edge> g = new UndirectedSparseGraph<>();
2828
for (String[] e : edges) {
29-
Edge edge = new Edge("f", e[0], e[1]);
3029
if (!g.containsVertex(e[0])) g.addVertex(e[0]);
3130
if (!g.containsVertex(e[1])) g.addVertex(e[1]);
32-
g.addEdge(edge, e[0], e[1]);
31+
// Skip duplicate undirected pairs so the snapshot stays a simple graph.
32+
if (g.findEdge(e[0], e[1]) != null) continue;
33+
g.addEdge(new Edge("f", e[0], e[1]), e[0], e[1]);
3334
}
3435
return g;
3536
}
@@ -443,10 +444,13 @@ public void testNodeTrajectoryLimitedTo20() {
443444
@Test
444445
public void testDegreeEntropyIncreases() {
445446
List<Graph<String, Edge>> snapshots = new ArrayList<>();
446-
// Uniform degree -> varied degree
447-
snapshots.add(createGraph(new String[][]{{"A","B"},{"B","C"},{"C","D"},{"D","A"}})); // all degree 2
448-
snapshots.add(createGraph(new String[][]{{"A","B"},{"B","C"},{"C","D"},{"D","A"},{"A","C"}})); // A,C degree 3
449-
snapshots.add(createGraph(new String[][]{{"A","B"},{"B","C"},{"C","D"},{"D","A"},{"A","C"},{"A","D"},{"B","D"}}));
447+
// Snapshot 0: uniform degree (4-cycle, every vertex has degree 2 -> entropy 0).
448+
snapshots.add(createGraph(new String[][]{{"A","B"},{"B","C"},{"C","D"},{"D","A"}}));
449+
// Snapshot 1: add diagonal A-C, so A and C become degree 3, B and D stay 2.
450+
snapshots.add(createGraph(new String[][]{{"A","B"},{"B","C"},{"C","D"},{"D","A"},{"A","C"}}));
451+
// Snapshot 2: extend with a pendant E off A and a pendant F off B -> degree mix {1,1,2,3,3,4}.
452+
snapshots.add(createGraph(new String[][]{
453+
{"A","B"},{"B","C"},{"C","D"},{"D","A"},{"A","C"},{"A","E"},{"B","F"}}));
450454
GraphTemporalDynamicsEngine.TemporalDynamicsReport report = engine.analyze(snapshots);
451455
// First snapshot: all same degree -> entropy = 0 (one bin)
452456
assertEquals(0.0, report.snapshots.get(0).degreeEntropy, 0.01);

0 commit comments

Comments
 (0)