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
F15343440: D15413.id.diff
Sun, Mar 9, 10:56 PM
F15283963: D15413.diff
Tue, Mar 4, 8:59 AM
Unknown Object (File)
Mon, Mar 3, 7:41 AM
Unknown Object (File)
Mon, Mar 3, 7:01 AM
Unknown Object (File)
Wed, Feb 26, 8:54 PM
Unknown Object (File)
Fri, Feb 14, 4:19 AM
Unknown Object (File)
Feb 9 2025, 12:57 AM
Unknown Object (File)
Feb 9 2025, 12:57 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.