arogozine / LinqToTypeScript

LINQ to TypeScript
https://arogozine.github.io/linqtotypescript/
MIT License
139 stars 18 forks source link

orderBy<number> with default comparer uses string-based sort order #19

Open vgpro54321 opened 2 years ago

vgpro54321 commented 2 years ago

Hello Alex,

Another thing I ran into, not to miss. Maybe everybody who knows javascript is perfectly aware of it, but a default "comparer" sucks at comparing numbers.

For example, let x = [1, 2, 100]; x.sort(); console.log('sorted', x); prints 1 100 2

I am concluding array.sort uses toString for sorting. This was totally unexpected, and I suggest comparer should be made a mandatory argument.

Now, in regard to sort implementation, eh, why does it populate map of arrays instead of say quick sort? Quick look on referencesource

https://github.com/microsoft/referencesource/blob/master/System.Core/System/Linq/Enumerable.cs


    internal abstract class EnumerableSorter<TElement>
    {
        internal abstract void ComputeKeys(TElement[] elements, int count);

        internal abstract int CompareKeys(int index1, int index2);

        internal int[] Sort(TElement[] elements, int count) {
            ComputeKeys(elements, count);
            int[] map = new int[count];
            for (int i = 0; i < count; i++) map[i] = i;
            QuickSort(map, 0, count - 1);
            return map;
        }

        void QuickSort(int[] map, int left, int right) {
            do {
                int i = left;
                int j = right;
                int x = map[i + ((j - i) >> 1)];
                do {
                    while (i < map.Length && CompareKeys(x, map[i]) > 0) i++;
                    while (j >= 0 && CompareKeys(x, map[j]) < 0) j--;
                    if (i > j) break;
                    if (i < j) {
                        int temp = map[i];
                        map[i] = map[j];
                        map[j] = temp;
                    }
                    i++;
                    j--;
                } while (i <= j);
                if (j - left <= right - i) {
                    if (left < j) QuickSort(map, left, j);
                    left = i;
                }
                else {
                    if (i < right) QuickSort(map, i, right);
                    right = j;
                }
            } while (left < right);
        }
    }
arogozine commented 2 years ago

I believe you're looking for NumberComparer

The current implementation could use a bit of optimization.

I do accept PRs to InDev branch. Please ensure they pass unit tests and linting.