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
F13207038: D15413.diff
Wed, May 15, 8:27 PM
Unknown Object (File)
Tue, May 7, 7:05 AM
Unknown Object (File)
Fri, May 3, 4:27 AM
Unknown Object (File)
Mon, Apr 29, 4:32 PM
Unknown Object (File)
Thu, Apr 25, 12:34 AM
Unknown Object (File)
Mon, Apr 22, 5:02 PM
Unknown Object (File)
Wed, Apr 17, 2:38 PM
Unknown Object (File)
Apr 7 2024, 2:39 AM
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.