[pull] master from trekhleb:master

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']);
+  });
 });

src/data-structures/graph/Graph.js

@@ -144,14 +144,19 @@ 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);
 
       // 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);
     });
 

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;
   }
 

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');

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');
+  });
 });