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 3379ca75fb..1fdabab9bf 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/Graph.js b/src/data-structures/graph/Graph.js index 9a9194f2e4..e021968806 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); }); diff --git a/src/data-structures/graph/GraphEdge.js b/src/data-structures/graph/GraphEdge.js index c0002f0f8b..76eb9dc019 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 111f4ad495..11c7a6c31d 100644 --- a/src/data-structures/graph/__test__/Graph.test.js +++ b/src/data-structures/graph/__test__/Graph.test.js @@ -318,6 +318,55 @@ 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 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 18dc731791..893e5209ba 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'); + }); });