Add sort in memory to Arrays library

[Amxx]

Replaces #3520 that is old. This comment remains relevant.

Fixes #3490
Fixes LIB-1189

IMO the approach should be to provide a simple/straight forward implementation that works. We can refine it/replace it down the line if we find a more efficient approach. Backward compatibility should not cause issues.

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

🦋 Changeset detected

Latest commit: 4336e2e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ernestognw]

LGTM

[ernestognw]

Before approving I want to clarify why we're picking the first element as the pivot.

I'm pending to dig more into this comment but I'd like to know if you got to a conclusion based on a previous discussion.

[Amxx]

Before approving I want to clarify why we're picking the first element as the pivot.

There are a few elements of answer, but the bottom line is: its simpler that way, and there is no reason to think its a bad choice.

Lets get into the issue:

A each step, the subroutine sort a subarray by choosing a pivot. Putting all the values that are smaller on one side, all the values that are bigger on the other, and run recursivelly on both "sides". The pivot is placed correctly for sure. The other values are "grouped" to be sorted. Mathematically, the best pivot is on that divides the array in two sub-arrays of equal size. Said otherwise, the best pivot is the median.
But if we wanted to get the median ... we basically need to sort the array, which is our goal.

We have tree options:

1. Try go be smart, and chose the pivot based on some algorithm
2. Chose the pivot randomnly
3. Take a specific element (first one/last one/...)

The first option requires reading the values in a loop in order to make some decision. It means reading the array, which comes at a cost. I'm not conviced that choosing a "good" pivot would save move gas (due to the nice "split" it would produce) than the cost involved in choosing the good pivot

If the array is shuffled, there is no reason to think that point 2 or 3 would be better. You have the same probability of getting a good or bad pivot. So it comes down to practicality.

Since you don't want to have to pivot "in the way", a common approach is to set it asside, sort the rest of the array (into the 2 sections discussed earlier), and then put the pivot in the right place. The easiest way to set the pivot asside, is to put it at the beginning (or at the end), and then inverse it with the last element smaller than the pivot (or the first element bigger than the pivot). If you take your pivot randomly, putting it asside costs you one swap operation. If you choose the first (or the last) its already placed somewhere nice.

So the bottom line is:

• using the first element is easy in terms of code
• its statistically not a better or worst choice than any other randomly choosen pivot
• you could do better by choosing a good pivot, but making this choice has a cost.

[ernestognw]

So the bottom line is:

• using the first element is easy in terms of code
• its statistically not a better or worst choice than any other randomly choosen pivot
• you could do better by choosing a good pivot, but making this choice has a cost.

Great, I agree with this conclusions. I know there are other strategies to heuristically determine which element to use as the pivot.

I think it's worth keeping in mind that there may be use cases where specifying the pivot as part of the params of _quickSort. As you mentioned, it requires reading the values in the array, which is costly, but surely there may be a threshold at which it still makes sense (e.g. if guaranteed to have 20 elements, reading 20 may be better than applying the quicksort right away).

Right now it's fine since the function is private anyway.