Fix correctness bugs found in a full-codebase audit

README.md

@@ -342,6 +342,7 @@ Below is the list of some of the most used Big O notations and their performance
 | **Array**               | 1         | n         | n         | n         |           |
 | **Stack**               | n         | n         | 1         | 1         |           |
 | **Queue**               | n         | n         | 1         | 1         |           |
+| **Deque**               | n         | n         | 1         | 1         | Insertion and deletion are O(1) at both the front and the back |
 | **Linked List**         | n         | n         | 1         | n         |           |
 | **Hash Table**          | -         | n         | n         | n         | In case of perfect hash function costs would be O(1) |
 | **Binary Search Tree**  | n         | n         | n         | n         | In case of balanced tree costs would be O(log(n)) |
@@ -357,7 +358,7 @@ Below is the list of some of the most used Big O notations and their performance
 | **Bubble sort**       | n               | n<sup>2</sup>       | n<sup>2</sup>       | 1         | Yes       |           |
 | **Insertion sort**    | n               | n<sup>2</sup>       | n<sup>2</sup>       | 1         | Yes       |           |
 | **Selection sort**    | n<sup>2</sup>   | n<sup>2</sup>       | n<sup>2</sup>       | 1         | No        |           |
-| **Heap sort**         | n&nbsp;log(n)   | n&nbsp;log(n)       | n&nbsp;log(n)       | 1         | No        |           |
+| **Heap sort**         | n&nbsp;log(n)   | n&nbsp;log(n)       | n&nbsp;log(n)       | 1         | No        | Classic heapsort is in-place; this repo's implementation copies the items into a heap and thus uses O(n) memory |
 | **Merge sort**        | n&nbsp;log(n)   | n&nbsp;log(n)       | n&nbsp;log(n)       | n         | Yes       |           |
 | **Quick sort**        | n&nbsp;log(n)   | n&nbsp;log(n)       | n<sup>2</sup>       | log(n)    | No        | Quicksort is usually done in-place with O(log(n)) stack space |
 | **Shell sort**        | n&nbsp;log(n)   | depends on gap sequence   | n&nbsp;(log(n))<sup>2</sup>  | 1         | No         |           |

src/algorithms/cryptography/hill-cipher/hillCipher.js

@@ -23,7 +23,7 @@ const generateKeyMatrix = (keyString) => {
     // Callback to get a value of each matrix cell.
     // The order the matrix is being filled in is from left to right, from top to bottom.
     () => {
-      // A → 0, B → 1, ..., a → 32, b → 33, ...
+      // A → 0, B → 1, ..., Z → 25
       const charCodeShifted = (keyString.codePointAt(keyStringIndex)) % alphabetCodeShift;
       keyStringIndex += 1;
       return charCodeShifted;
@@ -63,8 +63,11 @@ export function hillCipherEncrypt(message, keyString) {
     throw new Error('The message and key string can only contain letters');
   }
 
-  const keyMatrix = generateKeyMatrix(keyString);
-  const messageVector = generateMessageVector(message);
+  // The cipher works with the 26-letter alphabet where A → 0, ..., Z → 25.
+  // Normalize the case so that lowercase letters map to the same numbers
+  // as their uppercase counterparts.
+  const keyMatrix = generateKeyMatrix(keyString.toUpperCase());
+  const messageVector = generateMessageVector(message.toUpperCase());
 
   // keyString.length must equal to square of message.length
   if (keyMatrix.length !== message.length) {

src/algorithms/cryptography/polynomial-hash/PolynomialHash.js

@@ -39,7 +39,8 @@ export default class PolynomialHash {
    * Recalculates the hash representation of a word so that it isn't
    * necessary to traverse the whole word again.
    *
-   * Time complexity: O(1).
+   * Time complexity: O(word.length) because of the multiplier
+   * re-calculation loop (hash update itself takes O(1) time).
    *
    * @param {number} prevHash
    * @param {string} prevWord
@@ -49,11 +50,16 @@ export default class PolynomialHash {
   roll(prevHash, prevWord, newWord) {
     let hash = prevHash;
 
-    const prevValue = this.charToNumber(prevWord[0]);
-    const newValue = this.charToNumber(newWord[newWord.length - 1]);
+    // Use code points (and not UTF-16 code units) to be consistent with
+    // the hash() method and to support surrogate pairs (astral symbols).
+    const prevWordChars = Array.from(prevWord);
+    const newWordChars = Array.from(newWord);
+
+    const prevValue = this.charToNumber(prevWordChars[0]);
+    const newValue = this.charToNumber(newWordChars[newWordChars.length - 1]);
 
     let prevValueMultiplier = 1;
-    for (let i = 1; i < prevWord.length; i += 1) {
+    for (let i = 1; i < prevWordChars.length; i += 1) {
       prevValueMultiplier *= this.base;
       prevValueMultiplier %= this.modulus;
     }

src/algorithms/cryptography/polynomial-hash/README.md

@@ -37,9 +37,9 @@ The *Rabin–Karp string search algorithm* is often explained using a very simpl
 rolling hash function that only uses multiplications and 
 additions - **polynomial rolling hash**:
 
-> H(s<sub>0</sub>, s<sub>1</sub>, ..., s<sub>k</sub>) = s<sub>0</sub> * p<sup>k-1</sup> + s<sub>1</sub> * p<sup>k-2</sup> + ... + s<sub>k</sub> * p<sup>0</sup>
+> H(s<sub>0</sub>, s<sub>1</sub>, ..., s<sub>k</sub>) = s<sub>0</sub> * p<sup>k</sup> + s<sub>1</sub> * p<sup>k-1</sup> + ... + s<sub>k</sub> * p<sup>0</sup>
 
-where `p` is a constant, and *(s<sub>1</sub>, ... , s<sub>k</sub>)* are the input
+where `p` is a constant, and *(s<sub>0</sub>, ... , s<sub>k</sub>)* are the input
 characters.
 
 For example we can convert short strings to key numbers by multiplying digit codes by 
@@ -50,7 +50,7 @@ by calculating:
 
 In order to avoid manipulating huge `H` values, all math is done modulo `M`.
 
-> H(s<sub>0</sub>, s<sub>1</sub>, ..., s<sub>k</sub>) = (s<sub>0</sub> * p<sup>k-1</sup> + s<sub>1</sub> * p<sup>k-2</sup> + ... + s<sub>k</sub> * p<sup>0</sup>) mod M
+> H(s<sub>0</sub>, s<sub>1</sub>, ..., s<sub>k</sub>) = (s<sub>0</sub> * p<sup>k</sup> + s<sub>1</sub> * p<sup>k-1</sup> + ... + s<sub>k</sub> * p<sup>0</sup>) mod M
 
 A careful choice of the parameters `M`, `p` is important to obtain “good”
 properties of the hash function, i.e., low collision rate.
@@ -106,7 +106,7 @@ function hash(key, arraySize) {
 Polynomial hashing has a rolling property: the fingerprints can be updated 
 efficiently when symbols are added or removed at the ends of the string
 (provided that an array of powers of p modulo M of sufficient length is stored).
-The popular Rabin–Karp pattern matching algorithm is based on this property
+The popular Rabin–Karp pattern matching algorithm is based on this property.
 
 ## References
 

src/algorithms/cryptography/rail-fence-cipher/__test__/railFenceCipher.test.js

@@ -40,4 +40,12 @@ describe('railFenceCipher', () => {
       'THEYAREATTACKINGFROMTHENORTH',
     );
   });
+
+  it('leaves the message unchanged when there is a single rail', () => {
+    // A single-rail fence can't zig-zag, so the cipher is the identity.
+    expect(encodeRailFenceCipher('abc', 1)).toBe('abc');
+    expect(decodeRailFenceCipher('abc', 1)).toBe('abc');
+    expect(encodeRailFenceCipher('', 1)).toBe('');
+    expect(decodeRailFenceCipher('', 1)).toBe('');
+  });
 });

src/algorithms/cryptography/rail-fence-cipher/railFenceCipher.js

@@ -201,6 +201,12 @@ const decodeFence = (params) => {
  * @returns {string} - Encoded string
  */
 export const encodeRailFenceCipher = (string, railCount) => {
+  // A fence with a single rail (or no rails at all) can't zig-zag
+  // and thus leaves the message unchanged.
+  if (railCount <= 1) {
+    return string;
+  }
+
   const fence = buildFence(railCount);
 
   const filledFence = fillEncodeFence({
@@ -221,6 +227,12 @@ export const encodeRailFenceCipher = (string, railCount) => {
  * @returns {string} - Decoded string.
  */
 export const decodeRailFenceCipher = (string, railCount) => {
+  // A fence with a single rail (or no rails at all) can't zig-zag
+  // and thus leaves the message unchanged.
+  if (railCount <= 1) {
+    return string;
+  }
+
   const strLen = string.length;
   const emptyFence = buildFence(railCount);
   const filledFence = fillDecodeFence({

src/algorithms/graph/articulation-points/__test__/articulationPoints.test.js

@@ -229,4 +229,35 @@ describe('articulationPoints', () => {
     expect(articulationPointsSet.length).toBe(1);
     expect(articulationPointsSet[0].getKey()).toBe(vertexC.getKey());
   });
+
+  it('should find articulation points in all components of disconnected graph', () => {
+    const vertexA = new GraphVertex('A');
+    const vertexB = new GraphVertex('B');
+    const vertexC = new GraphVertex('C');
+    const vertexD = new GraphVertex('D');
+    const vertexE = new GraphVertex('E');
+    const vertexF = new GraphVertex('F');
+
+    // First component is a path "A - B - C" with articulation point B.
+    const edgeAB = new GraphEdge(vertexA, vertexB);
+    const edgeBC = new GraphEdge(vertexB, vertexC);
+
+    // Second component is a path "D - E - F" with articulation point E.
+    const edgeDE = new GraphEdge(vertexD, vertexE);
+    const edgeEF = new GraphEdge(vertexE, vertexF);
+
+    const graph = new Graph();
+
+    graph
+      .addEdge(edgeAB)
+      .addEdge(edgeBC)
+      .addEdge(edgeDE)
+      .addEdge(edgeEF);
+
+    const articulationPointsSet = Object.values(articulationPoints(graph));
+
+    expect(articulationPointsSet.length).toBe(2);
+    expect(articulationPointsSet[0].getKey()).toBe(vertexB.getKey());
+    expect(articulationPointsSet[1].getKey()).toBe(vertexE.getKey());
+  });
 });

src/algorithms/graph/articulation-points/articulationPoints.js

@@ -29,8 +29,9 @@ export default function articulationPoints(graph) {
   // Time needed to discover to the current vertex.
   let discoveryTime = 0;
 
-  // Peek the start vertex for DFS traversal.
-  const startVertex = graph.getAllVertices()[0];
+  // Root vertex of the graph component that is currently being traversed.
+  // The graph might be disconnected thus every component will have its own root.
+  let startVertex = null;
 
   const dfsCallbacks = {
     /**
@@ -106,8 +107,16 @@ export default function articulationPoints(graph) {
     },
   };
 
-  // Do Depth First Search traversal over submitted graph.
-  depthFirstSearch(graph, startVertex, dfsCallbacks);
+  // Do Depth First Search traversal over all graph components since the graph
+  // might be disconnected. Every unvisited vertex starts the traversal of its
+  // component and becomes the root of that component. Notice that discovery
+  // time keeps on incrementing from component to component.
+  graph.getAllVertices().forEach((vertex) => {
+    if (!visitedSet[vertex.getKey()]) {
+      startVertex = vertex;
+      depthFirstSearch(graph, startVertex, dfsCallbacks);
+    }
+  });
 
   return articulationPointsSet;
 }

src/algorithms/graph/bellman-ford/__test__/bellmanFord.test.js

@@ -114,4 +114,28 @@ describe('bellmanFord', () => {
     expect(previousVertices.A.getKey()).toBe('D');
     expect(previousVertices.D.getKey()).toBe('E');
   });
+
+  it('should throw an error in case of negative weight cycle', () => {
+    function bellmanFordNegativeCycle() {
+      const vertexA = new GraphVertex('A');
+      const vertexB = new GraphVertex('B');
+      const vertexC = new GraphVertex('C');
+
+      // Cycle "B - C - B" has total weight of (-3 + 1) = -2. It means that
+      // the shortest paths may be infinitely improved by walking the cycle.
+      const edgeAB = new GraphEdge(vertexA, vertexB, 1);
+      const edgeBC = new GraphEdge(vertexB, vertexC, -3);
+      const edgeCB = new GraphEdge(vertexC, vertexB, 1);
+
+      const graph = new Graph(true);
+      graph
+        .addEdge(edgeAB)
+        .addEdge(edgeBC)
+        .addEdge(edgeCB);
+
+      bellmanFord(graph, vertexA);
+    }
+
+    expect(bellmanFordNegativeCycle).toThrow('Graph contains a negative weight cycle');
+  });
 });

src/algorithms/graph/bellman-ford/bellmanFord.js

@@ -38,6 +38,20 @@ export default function bellmanFord(graph, startVertex) {
     });
   }
 
+  // Do one more relaxation round over all the edges. After (|V| - 1) iterations
+  // all the distances must be final. Thus if any distance may still be improved
+  // then the graph must contain a negative weight cycle.
+  Object.keys(distances).forEach((vertexKey) => {
+    const vertex = graph.getVertexByKey(vertexKey);
+
+    graph.getNeighbors(vertex).forEach((neighbor) => {
+      const edge = graph.findEdge(vertex, neighbor);
+      if (distances[vertex.getKey()] + edge.weight < distances[neighbor.getKey()]) {
+        throw new Error('Graph contains a negative weight cycle');
+      }
+    });
+  });
+
   return {
     distances,
     previousVertices,

src/algorithms/graph/breadth-first-search/README.md

@@ -1,7 +1,7 @@
 # Breadth-First Search (BFS)
 
-Breadth-first search (BFS) is an algorithm for traversing, 
-searching tree, or graph data structures. It starts at
+Breadth-first search (BFS) is an algorithm for traversing 
+or searching tree or graph data structures. It starts at
 the tree root (or some arbitrary node of a graph, sometimes 
 referred to as a 'search key') and explores the neighbor
 nodes first, before moving to the next level neighbors.

src/algorithms/graph/breadth-first-search/__test__/breadthFirstSearch.test.js

@@ -171,4 +171,35 @@ describe('breadthFirstSearch', () => {
       expect(params.previousVertex).toEqual(leaveVertexParamsMap[callIndex].previousVertex);
     }
   });
+
+  it('should enter each vertex of directed cycle exactly once by default', () => {
+    const graph = new Graph(true);
+
+    const vertexA = new GraphVertex('A');
+    const vertexB = new GraphVertex('B');
+    const vertexC = new GraphVertex('C');
+
+    // Directed triangle "A -> B -> C -> A" that leads back to the start vertex.
+    const edgeAB = new GraphEdge(vertexA, vertexB);
+    const edgeBC = new GraphEdge(vertexB, vertexC);
+    const edgeCA = new GraphEdge(vertexC, vertexA);
+
+    graph
+      .addEdge(edgeAB)
+      .addEdge(edgeBC)
+      .addEdge(edgeCA);
+
+    const enterVertexCallback = jest.fn();
+
+    breadthFirstSearch(graph, vertexA, {
+      enterVertex: enterVertexCallback,
+    });
+
+    expect(enterVertexCallback).toHaveBeenCalledTimes(3);
+
+    const enteredVertices = enterVertexCallback.mock.calls.map(
+      (call) => call[0].currentVertex.getKey(),
+    );
+    expect(enteredVertices).toEqual(['A', 'B', 'C']);
+  });
 });

src/algorithms/graph/breadth-first-search/breadthFirstSearch.js

@@ -24,7 +24,10 @@ function initCallbacks(callbacks = {}) {
   const allowTraversalCallback = (
     () => {
       const seen = {};
-      return ({ nextVertex }) => {
+      return ({ currentVertex, nextVertex }) => {
+        // Mark current vertex as seen as well so that traversal would never
+        // get back to it (i.e. when the cycle leads back to the start vertex).
+        seen[currentVertex.getKey()] = true;
         if (!seen[nextVertex.getKey()]) {
           seen[nextVertex.getKey()] = true;
           return true;

src/algorithms/graph/bridges/__test__/graphBridges.test.js

@@ -200,4 +200,35 @@ describe('graphBridges', () => {
     expect(bridges.length).toBe(1);
     expect(bridges[0].getKey()).toBe(edgeCD.getKey());
   });
+
+  it('should find bridges in all components of disconnected graph', () => {
+    const vertexA = new GraphVertex('A');
+    const vertexB = new GraphVertex('B');
+    const vertexC = new GraphVertex('C');
+    const vertexD = new GraphVertex('D');
+    const vertexE = new GraphVertex('E');
+
+    // First component "A - B" consists of one bridge.
+    const edgeAB = new GraphEdge(vertexA, vertexB);
+
+    // Second component "C - D - E" consists of two more bridges.
+    const edgeCD = new GraphEdge(vertexC, vertexD);
+    const edgeDE = new GraphEdge(vertexD, vertexE);
+
+    const graph = new Graph();
+
+    graph
+      .addEdge(edgeAB)
+      .addEdge(edgeCD)
+      .addEdge(edgeDE);
+
+    const bridges = Object.values(graphBridges(graph));
+
+    expect(bridges.length).toBe(3);
+    expect(bridges.map((bridge) => bridge.getKey()).sort()).toEqual([
+      edgeAB.getKey(),
+      edgeCD.getKey(),
+      edgeDE.getKey(),
+    ].sort());
+  });
 });

src/algorithms/graph/bridges/graphBridges.js

@@ -24,9 +24,6 @@ export default function graphBridges(graph) {
   // Time needed to discover to the current vertex.
   let discoveryTime = 0;
 
-  // Peek the start vertex for DFS traversal.
-  const startVertex = graph.getAllVertices()[0];
-
   const dfsCallbacks = {
     /**
      * @param {GraphVertex} currentVertex
@@ -88,8 +85,14 @@ export default function graphBridges(graph) {
     },
   };
 
-  // Do Depth First Search traversal over submitted graph.
-  depthFirstSearch(graph, startVertex, dfsCallbacks);
+  // Do Depth First Search traversal over all graph components since the graph
+  // might be disconnected. Every unvisited vertex starts the traversal of its
+  // own component.
+  graph.getAllVertices().forEach((startVertex) => {
+    if (!visitedSet[startVertex.getKey()]) {
+      depthFirstSearch(graph, startVertex, dfsCallbacks);
+    }
+  });
 
   return bridges;
 }