From d60eac8a4db572e1dfcabfeaaa881e7e392aa246 Mon Sep 17 00:00:00 2001 From: krm35 Date: Thu, 3 Aug 2023 18:50:16 +0200 Subject: [PATCH 1/4] Fix bad edge creation on shortestPaths function --- src/index.ts | 16 +++++++++------- test.js | 11 +++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/index.ts b/src/index.ts index d5c6e4e..bd4e5ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -397,9 +397,14 @@ export function Graph(serialized?: Serialized) { removedEdges = [], weight = path.weight; while (weight) { - removeEdge(path[0], path[1]); - removeEdge(path[1], path[0]); - removedEdges.push([path[0], path[1]]); + if (hasEdge(path[0], path[1])) { + removeEdge(path[0], path[1]); + removedEdges.push([path[0], path[1]]); + } + if (hasEdge(path[1], path[0])) { + removeEdge(path[1], path[0]); + removedEdges.push([path[1], path[0]]); + } try { path = shortestPath(source, destination); if (!path.weight || weight < path.weight) break; @@ -408,10 +413,7 @@ export function Graph(serialized?: Serialized) { break; } } - for (const [u, v] of removedEdges) { - addEdge(u, v); - addEdge(v, u); - } + for (const [u, v] of removedEdges) addEdge(u, v); return paths; } diff --git a/test.js b/test.js index 922e3f4..3ac5bef 100644 --- a/test.js +++ b/test.js @@ -427,7 +427,7 @@ describe("Graph", function () { ); }); - it("Should compute shortest paths on six edges.", function () { + it("Should compute shortest paths.", function () { var graph = Graph() .addEdge("a", "b") .addEdge("b", "c") @@ -444,6 +444,9 @@ describe("Graph", function () { const nodes = ["a", "b", "c", "d", "e", "f"]; assert.equal(graph.nodes().length, nodes.length); nodes.forEach((node) => assert(contains(graph.nodes(), node))); + // check edges are still there and no new have been added + assert.deepEqual(graph.hasEdge("a", "b"), true); + assert.deepEqual(graph.hasEdge("b", "a"), false); }); }); @@ -458,11 +461,7 @@ describe("Graph", function () { }); function contains(arr, item) { - return ( - arr.filter(function (d) { - return d === item; - }).length > 0 - ); + return arr.includes(item); } function comesBefore(arr, a, b) { From 1e4d5d2d8d093052f8a0c912f0c7f7959a53c25e Mon Sep 17 00:00:00 2001 From: krm35 Date: Thu, 3 Aug 2023 18:56:16 +0200 Subject: [PATCH 2/4] Check the graph has not changed after shortestPaths call --- test.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test.js b/test.js index 3ac5bef..e77eb09 100644 --- a/test.js +++ b/test.js @@ -436,17 +436,21 @@ describe("Graph", function () { .addEdge("a", "e") .addEdge("e", "f") .addEdge("f", "c"); + const serializedGraph = graph.serialize(); assert.deepEqual(graph.shortestPaths("a", "c"), [ withWeight(["a", "b", "c"], 2), withWeight(["a", "d", "c"], 2), ]); - // need to check nodes are still present because we remove them to get all shortest paths - const nodes = ["a", "b", "c", "d", "e", "f"]; - assert.equal(graph.nodes().length, nodes.length); - nodes.forEach((node) => assert(contains(graph.nodes(), node))); - // check edges are still there and no new have been added - assert.deepEqual(graph.hasEdge("a", "b"), true); - assert.deepEqual(graph.hasEdge("b", "a"), false); + // check graph has not changed + const postSerializedGraph = graph.serialize(); + assert.equal( + postSerializedGraph.links.length, + serializedGraph.links.length, + ); + assert.equal( + postSerializedGraph.nodes.length, + serializedGraph.nodes.length, + ); }); }); From ccb022305cac68bdafb901a28d76802b272acb4f Mon Sep 17 00:00:00 2001 From: krm35 Date: Thu, 3 Aug 2023 19:47:45 +0200 Subject: [PATCH 3/4] Lint and add edgeWeight --- src/index.ts | 25 +++++++++++++------------ test.js | 8 ++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/index.ts b/src/index.ts index bd4e5ac..9c8a3db 100644 --- a/src/index.ts +++ b/src/index.ts @@ -178,7 +178,7 @@ export function Graph(serialized?: Serialized) { function depthFirstSearch( sourceNodes?: NodeId[], includeSourceNodes: boolean = true, - errorOnCycle: boolean = false + errorOnCycle: boolean = false, ) { if (!sourceNodes) { sourceNodes = nodes(); @@ -284,7 +284,7 @@ export function Graph(serialized?: Serialized) { // Cormen et al. "Introduction to Algorithms" 3rd Ed. p. 613 function topologicalSort( sourceNodes?: NodeId[], - includeSourceNodes: boolean = true + includeSourceNodes: boolean = true, ) { return depthFirstSearch(sourceNodes, includeSourceNodes, true).reverse(); } @@ -394,17 +394,18 @@ export function Graph(serialized?: Serialized) { function shortestPaths(source: NodeId, destination: NodeId) { let path = shortestPath(source, destination); const paths = [path], - removedEdges = [], + removedEdges: any = [], weight = path.weight; while (weight) { - if (hasEdge(path[0], path[1])) { - removeEdge(path[0], path[1]); - removedEdges.push([path[0], path[1]]); - } - if (hasEdge(path[1], path[0])) { - removeEdge(path[1], path[0]); - removedEdges.push([path[1], path[0]]); - } + [ + [path[0], path[1]], + [path[1], path[0]], + ].forEach(([u, v]) => { + if (hasEdge(u, v)) { + removedEdges.push({ u, v, weight: getEdgeWeight(u, v) }); + removeEdge(u, v); + } + }); try { path = shortestPath(source, destination); if (!path.weight || weight < path.weight) break; @@ -413,7 +414,7 @@ export function Graph(serialized?: Serialized) { break; } } - for (const [u, v] of removedEdges) addEdge(u, v); + for (const { u, v, weight } of removedEdges) addEdge(u, v, weight); return paths; } diff --git a/test.js b/test.js index e77eb09..60210ea 100644 --- a/test.js +++ b/test.js @@ -378,7 +378,7 @@ describe("Graph", function () { var graph = Graph().addEdge("a", "b").addEdge("b", "c"); assert.deepEqual( graph.shortestPath("a", "c"), - withWeight(["a", "b", "c"], 2) + withWeight(["a", "b", "c"], 2), ); }); @@ -396,11 +396,11 @@ describe("Graph", function () { assert.deepEqual( graph.shortestPath("s", "z"), - withWeight(["s", "y", "z"], 5 + 2) + withWeight(["s", "y", "z"], 5 + 2), ); assert.deepEqual( graph.shortestPath("s", "x"), - withWeight(["s", "y", "t", "x"], 5 + 3 + 1) + withWeight(["s", "y", "t", "x"], 5 + 3 + 1), ); }); @@ -423,7 +423,7 @@ describe("Graph", function () { var graph = Graph().addEdge("a", "b").addEdge("b", "c").addEdge("d", "e"); assert.deepEqual( graph.shortestPath("a", "c"), - withWeight(["a", "b", "c"], 2) + withWeight(["a", "b", "c"], 2), ); }); From e3cc716f95eed16ed09f71607663091a874c7a06 Mon Sep 17 00:00:00 2001 From: krm35 Date: Fri, 4 Aug 2023 19:43:03 +0200 Subject: [PATCH 4/4] Optimize performance and add proper types --- src/index.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index 9c8a3db..941fb17 100644 --- a/src/index.ts +++ b/src/index.ts @@ -394,18 +394,22 @@ export function Graph(serialized?: Serialized) { function shortestPaths(source: NodeId, destination: NodeId) { let path = shortestPath(source, destination); const paths = [path], - removedEdges: any = [], + removedEdges: { u: NodeId; v: NodeId; weight: EdgeWeight }[] = [], weight = path.weight; while (weight) { - [ - [path[0], path[1]], - [path[1], path[0]], - ].forEach(([u, v]) => { - if (hasEdge(u, v)) { - removedEdges.push({ u, v, weight: getEdgeWeight(u, v) }); - removeEdge(u, v); - } - }); + const u = path[0]; + const v = path[1]; + + if (hasEdge(u, v)) { + removedEdges.push({ u, v, weight: getEdgeWeight(u, v) }); + removeEdge(u, v); + } + + if (hasEdge(v, u)) { + removedEdges.push({ u: v, v: u, weight: getEdgeWeight(v, u) }); + removeEdge(v, u); + } + try { path = shortestPath(source, destination); if (!path.weight || weight < path.weight) break;