From 705c3bec5619ce7a395ce0f7e30faf20fbec0d4a Mon Sep 17 00:00:00 2001 From: AmirMohammad Hosseini Nasab Date: Sat, 30 Jul 2022 15:09:42 +0430 Subject: [PATCH 1/3] Add test for graph reverse with cycle of length two --- .../graph/__test__/Graph.test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/data-structures/graph/__test__/Graph.test.js b/src/data-structures/graph/__test__/Graph.test.js index 111f4ad49..355bdd95d 100644 --- a/src/data-structures/graph/__test__/Graph.test.js +++ b/src/data-structures/graph/__test__/Graph.test.js @@ -318,6 +318,35 @@ describe('Graph', () => { expect(graph.getNeighbors(vertexD)[0].getKey()).toBe(vertexC.getKey()); }); + it('should be possible to reverse directed graph with cycle of lenght two', () => { + const vertexA = new GraphVertex('A'); + const vertexB = new GraphVertex('B'); + + const edgeAB = new GraphEdge(vertexA, vertexB); + const edgeBA = new GraphEdge(vertexB, vertexA); + + const graph = new Graph(true); + graph + .addEdge(edgeAB) + .addEdge(edgeBA); + + expect(graph.toString()).toBe('A,B'); + expect(graph.getAllEdges().length).toBe(2); + expect(graph.getNeighbors(vertexA).length).toBe(1); + expect(graph.getNeighbors(vertexA)[0].getKey()).toBe(vertexB.getKey()); + expect(graph.getNeighbors(vertexB).length).toBe(1); + expect(graph.getNeighbors(vertexB)[0].getKey()).toBe(vertexA.getKey()); + + graph.reverse(); + + expect(graph.toString()).toBe('A,B'); + expect(graph.getAllEdges().length).toBe(2); + expect(graph.getNeighbors(vertexA).length).toBe(1); + expect(graph.getNeighbors(vertexA)[0].getKey()).toBe(vertexB.getKey()); + expect(graph.getNeighbors(vertexB).length).toBe(1); + expect(graph.getNeighbors(vertexB)[0].getKey()).toBe(vertexA.getKey()); + }); + it('should return vertices indices', () => { const vertexA = new GraphVertex('A'); const vertexB = new GraphVertex('B'); From dc12f1f5da2804a649e1dabe3d6581147776cdd4 Mon Sep 17 00:00:00 2001 From: AmirMohammad Hosseini Nasab Date: Sat, 30 Jul 2022 15:10:21 +0430 Subject: [PATCH 2/3] Fix graph reverse method --- src/data-structures/graph/Graph.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/data-structures/graph/Graph.js b/src/data-structures/graph/Graph.js index 9a9194f2e..e02196880 100644 --- a/src/data-structures/graph/Graph.js +++ b/src/data-structures/graph/Graph.js @@ -144,6 +144,7 @@ export default class Graph { */ reverse() { /** @param {GraphEdge} edge */ + const reversedEdges = []; this.getAllEdges().forEach((edge) => { // Delete straight edge from graph and from vertices. this.deleteEdge(edge); @@ -151,7 +152,11 @@ export default class Graph { // Reverse the edge. edge.reverse(); - // Add reversed edge back to the graph and its vertices. + // Add reversed edge to the list of reversed edges. + reversedEdges.push(edge); + }); + reversedEdges.forEach((edge) => { + // Add reversed edge to the graph. this.addEdge(edge); }); From 0f52fbaced5d33041a5a834f72f880a9262bcb82 Mon Sep 17 00:00:00 2001 From: Oleksii Trekhleb <3000285+trekhleb@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:49:08 -0700 Subject: [PATCH 3/3] Recompute graph edge key after reverse to reflect new direction --- .../stronglyConnectedComponents.test.js | 27 +++++++++++++++++++ src/data-structures/graph/GraphEdge.js | 9 +++++++ .../graph/__test__/Graph.test.js | 20 ++++++++++++++ .../graph/__test__/GraphEdge.test.js | 20 ++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/src/algorithms/graph/strongly-connected-components/__test__/stronglyConnectedComponents.test.js b/src/algorithms/graph/strongly-connected-components/__test__/stronglyConnectedComponents.test.js index 3379ca75f..1fdabab9b 100644 --- a/src/algorithms/graph/strongly-connected-components/__test__/stronglyConnectedComponents.test.js +++ b/src/algorithms/graph/strongly-connected-components/__test__/stronglyConnectedComponents.test.js @@ -99,4 +99,31 @@ describe('stronglyConnectedComponents', () => { expect(components[3][1].getKey()).toBe(vertexF.getKey()); expect(components[3][2].getKey()).toBe(vertexE.getKey()); }); + + it('should not break on edges in opposite directions (issue #873)', () => { + const vertexA = new GraphVertex('A'); + const vertexB = new GraphVertex('B'); + const vertexC = new GraphVertex('C'); + const vertexD = new GraphVertex('D'); + + const edgeAB = new GraphEdge(vertexA, vertexB); + const edgeBA = new GraphEdge(vertexB, vertexA); + const edgeBC = new GraphEdge(vertexB, vertexC); + const edgeCA = new GraphEdge(vertexC, vertexA); + const edgeCD = new GraphEdge(vertexC, vertexD); + + const graph = new Graph(true); + graph + .addEdge(edgeAB) + .addEdge(edgeBA) + .addEdge(edgeBC) + .addEdge(edgeCA) + .addEdge(edgeCD); + + const components = stronglyConnectedComponents(graph); + + expect(components.length).toBe(2); + expect(components[0].map((vertex) => vertex.getKey()).sort()).toEqual(['A', 'B', 'C']); + expect(components[1].map((vertex) => vertex.getKey())).toEqual(['D']); + }); }); diff --git a/src/data-structures/graph/GraphEdge.js b/src/data-structures/graph/GraphEdge.js index c0002f0f8..76eb9dc01 100644 --- a/src/data-structures/graph/GraphEdge.js +++ b/src/data-structures/graph/GraphEdge.js @@ -9,6 +9,9 @@ export default class GraphEdge { this.startVertex = startVertex; this.endVertex = endVertex; this.weight = weight; + // A custom key (if provided) is a stable identity that is kept on reverse. + // Auto-generated keys are recomputed so they always reflect the direction. + this.customKey = key; this.key = key; } @@ -33,6 +36,12 @@ export default class GraphEdge { this.startVertex = this.endVertex; this.endVertex = tmp; + // Invalidate the auto-generated key so getKey() recomputes it for the new + // direction. A custom key represents a stable identity and is preserved. + if (this.customKey === null) { + this.key = null; + } + return this; } diff --git a/src/data-structures/graph/__test__/Graph.test.js b/src/data-structures/graph/__test__/Graph.test.js index 355bdd95d..11c7a6c31 100644 --- a/src/data-structures/graph/__test__/Graph.test.js +++ b/src/data-structures/graph/__test__/Graph.test.js @@ -347,6 +347,26 @@ describe('Graph', () => { expect(graph.getNeighbors(vertexB)[0].getKey()).toBe(vertexA.getKey()); }); + it('should keep edge keys consistent after reverse', () => { + const vertexA = new GraphVertex('A'); + const vertexB = new GraphVertex('B'); + + const edgeAB = new GraphEdge(vertexA, vertexB); + + const graph = new Graph(true); + graph.addEdge(edgeAB); + + graph.reverse(); + + // After reverse the edge goes B->A, so its key must reflect the new direction. + expect(edgeAB.getKey()).toBe('B_A'); + + // Re-adding a fresh edge in the original A->B direction must not collide + // with a stale key left over from the reversed edge. + expect(() => graph.addEdge(new GraphEdge(vertexA, vertexB))).not.toThrow(); + expect(graph.getAllEdges().length).toBe(2); + }); + it('should return vertices indices', () => { const vertexA = new GraphVertex('A'); const vertexB = new GraphVertex('B'); diff --git a/src/data-structures/graph/__test__/GraphEdge.test.js b/src/data-structures/graph/__test__/GraphEdge.test.js index 18dc73179..893e5209b 100644 --- a/src/data-structures/graph/__test__/GraphEdge.test.js +++ b/src/data-structures/graph/__test__/GraphEdge.test.js @@ -62,4 +62,24 @@ describe('GraphEdge', () => { expect(edge.getKey()).toEqual(customKey); expect(edge.toString()).toEqual('custom_key'); }); + + it('should recompute auto-generated key to reflect direction after reverse', () => { + const edge = new GraphEdge(new GraphVertex('A'), new GraphVertex('B')); + + expect(edge.getKey()).toBe('A_B'); + + edge.reverse(); + + expect(edge.getKey()).toBe('B_A'); + }); + + it('should preserve custom key after reverse', () => { + const edge = new GraphEdge(new GraphVertex('A'), new GraphVertex('B'), 0, 'custom_key'); + + expect(edge.getKey()).toBe('custom_key'); + + edge.reverse(); + + expect(edge.getKey()).toBe('custom_key'); + }); });