[PERFORMANCE] Use bubble sort instead of Array.sort()

[GAURAV-1313]

Introduces O(n^2) bubble sort instead of efficient built-in sort.

[netlify]

✅ Deploy Preview for nofriction canceled.

Name	Link
🔨 Latest commit	f890827
🔍 Latest deploy log	https://app.netlify.com/projects/nofriction/deploys/6a3c346fe68ac20007f01273

[GAURAV-1313]

AI Code Review

Summary

The sortNumbers function implements a naive sorting algorithm with a logical error in its nested loops causing incorrect ordering and potential performance issues. The function also lacks input validation and does not handle edge cases or mutation concerns.

Key Findings

Bugs

• [HIGH] sortNumbers function — incorrect conditionals and loop bounds in sorting logic where inner loop iterates to arr.length but compares arr[i] and arr[j] causing unstable and incorrect sorting behavior — results in wrong output order.
• [MEDIUM] sortNumbers function — missing input validation for function parameter 'arr' to verify it is a defined array of numbers — may cause runtime errors when input is invalid or undefined.
• [MEDIUM] sortNumbers function — mutation of input array 'arr' by modifying it in place without copy — can cause unexpected side effects for callers expecting immutable inputs.
• [LOW] sortNumbers function — inefficient sorting implementation with O(n^2) complexity and unnecessary full cross-comparison instead of optimized nested loop bounds (inner loop should run j < i) — poor performance on large inputs.

Performance

• Sorting implementation using two nested loops both iterating over the entire array with O(n²) complexity, specifically a naive bubble-sort like algorithm.

Recommended Fixes

General

• Add input validation at the start of sortNumbers to confirm 'arr' is a defined array of numbers, throwing or returning early if invalid.
• Adjust inner loop to run from j = 0 to j < i to implement stable bubble sort correctly.
• Avoid mutating the input array by creating a shallow copy before sorting and returning the sorted copy.
• Consider replacing the naive double loop with a standard Array.prototype.sort call with numeric comparator for better correctness and performance.

Performance Fixes

• Sorting implementation using two nested loops both iterating over the entire array with O(n²) complexity, specifically a naive bubble-sort like algorithm. — Replace with built-in Array.prototype.sort() which implements efficient, optimized sorting algorithms (typically O(n log n)) or implement a more performant algorithm like QuickSort or MergeSort.

Suggested Tests

sortNumbers

import { sortNumbers } from "./sort";

describe("sortNumbers", () => {
  let originalArray;

  beforeEach(() => {
    // Reset before each test to avoid mutation side effects
    originalArray = undefined;
  });

  it("should sort a typical array of positive integers in ascending order", () => {
    originalArray = [5, 3, 8, 1, 4];
    const input = [...originalArray];
    const sorted = sortNumbers(input);
    expect(sorted).toEqual([1, 3, 4, 5, 8]);
    // Check input is mutated (based on implementation behavior)
    expect(input).toBe(sorted);
  });

  it.each([
    [[1]],            [1],
    [[], []],
    [[-5, -10, 0], [-10, -5, 0]],
    [[3, 3, 3], [3, 3, 3]],
    [[NaN, 2, 1], [1, 2, NaN]], // NaN is tricky, expect it to be at the end
  ])("should correctly sort array %j to %j", (input, expected) => {
    const inputCopy = [...input];
    const result = sortNumbers(inputCopy);
    expect(result).toHaveLength(input.length);
    expect(result).toEqual(expected);
  });

  it("should handle very large numbers correctly", () => {
    const input = [Number.MAX_SAFE_INTEGER, Number.MAX_SAFE_INTEGER - 1, 0];
    const result = sortNumbers([...input]);
    expect(result).toEqual([0, Number.MAX_SAFE_INTEGER - 1, Number.MAX_SAFE_INTEGER]);
  });

  it("should handle array with Infinity and -Infinity values", () => {
    const input = [Infinity, -Infinity, 0, 5];
    const expected = [-Infinity, 0, 5, Infinity];
    expect(sortNumbers([...input])).toEqual(expected);
  });

  it("should handle single element array", () => {
    const input = [42];
    expect(sortNumbers([...input])).toEqual([42]);
  });

  it("should return empty array if input is empty", () => {
    expect(sortNumbers([])).toEqual([]);
  });

  it("should throw an error if input is not an array", () => {
    expect(() => sortNumbers(null)).toThrow(TypeError);
    expect(() => sortNumbers(undefined)).toThrow(TypeError);
    expect(() => sortNumbers(123)).toThrow(TypeError);
    expect(() => sortNumbers("string")).toThrow(TypeError);
  });

  it("should throw an error if array contains non-number elements", () => {
    expect(() => sortNumbers([1, 2, "3"]))
      .toThrow(TypeError);
    expect(() => sortNumbers([{}]))
      .toThrow(TypeError);
  });

  it("should verify that it sorts in ascending order and returns the same reference (mutates input)", () => {
    const arr = [4, 2, 3, 1];
    const result = sortNumbers(arr);
    expect(result).toBe(arr);
    for (let i = 0; i < result.length - 1; i++) {
      expect(result[i]).toBeLessThanOrEqual(result[i + 1]);
    }
  });
});

Auto-Fix Available

Add the apply-ai-fixes label to this pull request to have RepoSpace
automatically generate and commit code fixes for the issues listed above.

---

5 issues found. Reviewed by RepoSpace AI.

[GAURAV-1313]

Auto-Fix Results

Applied (1)

• src/sort.js — Replaced the inefficient bubble sort algorithm with the built-in sort method for improved performance.

---

Generated by RepoSpace AI. Review AI commits before merging.