Page MenuHomePhabricator

Add `msortv()`, a stable, well-behaved sort method
ClosedPublic

Authored by epriestley on Mar 6 2016, 3:57 PM.
Tags
None
Referenced Files
F18227471: D15413.id.diff
Wed, Aug 20, 6:14 AM
F18216523: D15413.diff
Tue, Aug 19, 7:46 AM
F18089646: D15413.id37158.diff
Wed, Aug 6, 11:55 AM
F17953461: D15413.id.diff
Fri, Aug 1, 3:06 AM
F17949186: D15413.id37158.diff
Thu, Jul 31, 10:57 PM
F17942379: D15413.id37160.diff
Thu, Jul 31, 6:43 AM
Unknown Object (File)
Jun 1 2025, 11:44 PM
Unknown Object (File)
Jun 1 2025, 11:39 PM
Subscribers
None

Details

Summary

Ref T6861. We currently have three minor issues with sorting:

  • If types get mixed, PHP goes crazy. This is discussed in T6861.
  • The sort isn't guaranteed to be stable (identical input items are not reordered), but we'd probably always prefer a stable sort to a slightly more efficient one.
  • Interacting with sort keys is a mess of sprintf('~%020.020f', ...) nonsense.

Introduce msortv() and PhutilSortVector to solve these. The new msortv() is:

  • well-behaved (no craziness on mixed types); and
  • stable (input items with the same sort key are never reordered).

PhutilSortVector is a helper so you don't have to do the sprintf(...) garbage. Instead, classes will do this:

- return sprintf(
-   '~%020d%s',
-   $this->getOrder(),
-   $this->getName());

+ return id(new PhutilSortVector())
+   ->addInt($this->getOrder())
+   ->addString($this->getName());

This should be easier to read and much easier to get right, particularly for odd inputs at the edge of the range (like the most-negative integer).

Test Plan

Added and executed unit tests.

Diff Detail

Repository
rPHU libphutil
Branch
sort1
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/utils/utils.php:1264XHP45PHP Compatibility
Errorsrc/utils/utils.php:1264XHP45PHP Compatibility
Unit
Tests Passed
Build Status
Buildable 11020
Build 13623: Run Core Tests
Build 13622: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Add `msortv()`, a stable, well-behaved sort method.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

Particular motivation here is that I'm going to be sorting actions and panels, and those sorts need to be stable and I don't particularly want to write yet another sprintf('~%...') method.

chad edited edge metadata.
This revision is now accepted and ready to land.Mar 6 2016, 4:08 PM
This revision was automatically updated to reflect the committed changes.