Skip to content

fix: handle zero and negative numbers in RadixSort#2184

Open
huizixin wants to merge 2 commits into
trekhleb:masterfrom
huizixin:fix/radix-sort-negative-numbers
Open

fix: handle zero and negative numbers in RadixSort#2184
huizixin wants to merge 2 commits into
trekhleb:masterfrom
huizixin:fix/radix-sort-negative-numbers

Conversation

@huizixin

Copy link
Copy Markdown
Contributor

Bug Description

RadixSort.getLengthOfLongestElement() fails when the input array contains only zeros or negative numbers:

  • Math.log10(0) returns -Infinity, causing numPasses to be -Infinity
  • Math.log10(-N) returns NaN, causing numPasses to be NaN

In both cases, the sorting loop (for (let currentIndex = 0; currentIndex < numPasses; ...)) never executes, and the unsorted array is returned as-is.

Steps to Reproduce

const sorter = new RadixSort();
sorter.sort([-3, -1, -2]); // Returns [-3, -1, -2] (not sorted!)
sorter.sort([0, 0, 0]);    // Returns [0, 0, 0] (correct by accident)

Fix

Added an early return in getLengthOfLongestElement() to handle edge cases where the maximum value is 0 or negative:

getLengthOfLongestElement(array) {
    if (this.isArrayOfNumbers(array)) {
      const max = Math.max(...array);
      // Handle edge cases where max is 0 or negative
      if (max <= 0) return 1;
      return Math.floor(Math.log10(max)) + 1;
    }
    // ...
}

Testing

The existing test suite (SortTester.testSort) includes [3, 4, 2, 1, 0, 0, 4, 3, 4, 2] which works correctly. However, arrays of purely negative numbers are not tested and reveal this bug.

Note: Radix Sort has inherent limitations with negative numbers. A complete fix would require splitting the array by sign and merging results. This PR addresses the immediate NaN/-Infinity issue to prevent silent incorrect behavior.

huizixin and others added 2 commits June 12, 2026 15:08
…Element

Math.log10(0) returns -Infinity and Math.log10(-N) returns NaN, causing the sorting loop to never execute for arrays containing only zeros or negative numbers.

Added early return for edge cases where max value is 0 or negative.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants