Add function to sort the elements of an array.

[enzoh]

No description provided.

[nomeata]

Thanks! But this should better come with some test cases, right…

[chenyan-dfinity]

Base library uses { #lt; #eq; #gt; } as return type for compare function. See https://github.com/dfinity-lab/motoko-base/blob/master/src/Heap.mo#L13 as an example.

btw, you can already use HeapSort via the Heap module, but of course quick sort is faster in arrays before we start to implement pairing heap.

[kritzcreek]

Most standard libraries I know of (Java, Python, Haskell, Rust, ...) use a version of Mergesort (or Timsort) as their default sort implementations.

The main benefit there is that having a stable sort is the better default.

It's also to protect against DOS attacks as Quicksort degrades to O(n^2) if I can predict your pivot choices. Given that we don't have access to randomness that's even easier than it usually is.

I think adding a Quicksort as sortUnstable and adding a disclaimer about the potential n^2 is just fine, but I'd prefer our Array.sort to be a stable sort.

[enzoh]

@kritzcreek Can you implement it? I'm perfectly okay with swapping out for more efficient implementations as they become available. Right now I just need features.

[kritzcreek]

As I said, I don't mean to block this from being merged. I just think we ought to reserve the sortBy name for a stable sort.

I'm perfectly okay with swapping out for more efficient implementations as they become available.

It's not about efficiency, it's about correctness/semantics.

Can you implement it?

I might give it a try :)

[enzoh]

@kritzcreek https://dfinity.atlassian.net/browse/SDK-790

Issues adressed

[enzoh]

I would prefer to not have more than one sort function in the API. So, if the preservation of these semantics is a requirement, which is perfectly fine, then lets get the merge sort implemented. I need this to implement the key space module for my DHT.

[matthewhammer]

I think it's okay to have a non-stable sort for now if we make an issue to track this fact.

[matthewhammer]

Alternatively (in place of implementing sorting), you can build a Red-Black tree and then just iterate it using the basic API.

[matthewhammer]

This PR seems like it could be much worse than O(n log n) on common inputs.

Can this PR just use the red-black tree instead? (For loop over array elements, inserting them; then, iterate over the elements to get the sorted order in a buffer, etc.)

[enzoh]

The red-black tree module depends on the iterator module, which depends on this module, so no, I don't think that would work from a dependency standpoint.

[nomeata]

You can have Array/Internal.mo without sort, use that in Iterator, and then re-export the internal parts in Array.mo to break such cycles.