Skip to content

Restore stability for sorting, add tests #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Dec 29, 2020
6 changes: 0 additions & 6 deletions src/Data/Array.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,6 @@ exports.sortByImpl = (function () {
xs1[k++] = y;
++j;
}
else if (c === 0) {
xs1[k++] = x;
xs1[k++] = y;
++i;
++j;
}
else {
xs1[k++] = x;
++i;
Expand Down
6 changes: 5 additions & 1 deletion src/Data/Array.purs
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ foreign import scanr :: forall a b. (a -> b -> b) -> b -> Array a -> Array b
--------------------------------------------------------------------------------

-- | Sort the elements of an array in increasing order, creating a new array.
-- | Sorting is stable: the order of equal elements is preserved.
-- |
-- | ```purescript
-- | sort [2, -3, 1] = [-3, 1, 2]
Expand All @@ -800,6 +801,8 @@ sort xs = sortBy compare xs

-- | Sort the elements of an array in increasing order, where elements are
-- | compared using the specified partial ordering, creating a new array.
-- | Sorting is stable: the order of elements is preserved if they are equal
-- | according to the specified partial ordering.
-- |
-- | ```purescript
-- | compareLength a b = compare (length a) (length b)
Expand All @@ -813,7 +816,8 @@ sortBy comp = sortByImpl comp case _ of
LT -> -1

-- | Sort the elements of an array in increasing order, where elements are
-- | sorted based on a projection
-- | sorted based on a projection. Sorting is stable: the order of elements is
-- | preserved if they are equal according to the projection.
-- |
-- | ```purescript
-- | sortWith (_.age) [{name: "Alice", age: 42}, {name: "Bob", age: 21}]
Expand Down
5 changes: 2 additions & 3 deletions src/Data/Array/NonEmpty/Internal.purs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import Data.Foldable (class Foldable)
import Data.FoldableWithIndex (class FoldableWithIndex)
import Data.FunctorWithIndex (class FunctorWithIndex)
import Data.Ord (class Ord1)
import Data.Semigroup.Foldable (class Foldable1, foldMap1Default)
import Data.Semigroup.Foldable (class Foldable1, foldMap1DefaultL)
import Data.Semigroup.Traversable (class Traversable1, sequence1Default)
import Data.Traversable (class Traversable)
import Data.TraversableWithIndex (class TraversableWithIndex)
Expand Down Expand Up @@ -48,8 +48,7 @@ derive newtype instance foldableNonEmptyArray :: Foldable NonEmptyArray
derive newtype instance foldableWithIndexNonEmptyArray :: FoldableWithIndex Int NonEmptyArray

instance foldable1NonEmptyArray :: Foldable1 NonEmptyArray where
foldMap1 = foldMap1Default
fold1 = foldl1Impl (<>)
foldMap1 = foldMap1DefaultL
foldr1 = foldr1Impl
foldl1 = foldl1Impl

Expand Down
6 changes: 0 additions & 6 deletions src/Data/Array/ST.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,6 @@ exports.sortByImpl = (function () {
xs1[k++] = y;
++j;
}
else if (c === 0) {
xs1[k++] = x;
xs1[k++] = y;
++i;
++j;
}
else {
xs1[k++] = x;
++i;
Expand Down
10 changes: 7 additions & 3 deletions src/Data/Array/ST.purs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ empty = new
-- | Create a mutable copy of an immutable array.
foreign import thaw :: forall h a. Array a -> ST h (STArray h a)

-- | Sort a mutable array in place.
-- | Sort a mutable array in place. Sorting is stable: the order of equal
-- | elements is preserved.
sort :: forall a h. Ord a => STArray h a -> ST h (STArray h a)
sort = sortBy compare

Expand All @@ -101,7 +102,9 @@ foreign import shiftImpl
-> STArray h a
-> ST h (Maybe a)

-- | Sort a mutable array in place using a comparison function.
-- | Sort a mutable array in place using a comparison function. Sorting is
-- | stable: the order of elements is preserved if they are equal according to
-- | the comparison function.
sortBy
:: forall a h
. (a -> a -> Ordering)
Expand All @@ -119,7 +122,8 @@ foreign import sortByImpl
-> STArray h a
-> ST h (STArray h a)

-- | Sort a mutable array in place based on a projection.
-- | Sort a mutable array in place based on a projection. Sorting is stable: the
-- | order of elements is preserved if they are equal according to the projection.
sortWith
:: forall a b h
. Ord b
Expand Down
10 changes: 9 additions & 1 deletion test/Test/Data/Array.purs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Data.Foldable (for_, foldMapDefaultR, class Foldable, all, traverse_)
import Data.Traversable (scanl, scanr)
import Data.Maybe (Maybe(..), isNothing, fromJust)
import Data.Ord.Down (Down(..))
import Data.Tuple (Tuple(..))
import Data.Tuple (Tuple(..), fst)
import Data.Unfoldable (replicateA)
import Effect (Effect)
import Effect.Console (log)
Expand Down Expand Up @@ -288,9 +288,17 @@ testArray = do
log "sortBy should reorder a list into ascending order based on the result of a comparison function"
assert $ A.sortBy (flip compare) [1, 3, 2, 5, 6, 4] == [6, 5, 4, 3, 2, 1]

log "sortBy should not reorder elements that are equal according to a comparison function"
let s1 = map (Tuple "a") (A.range 1 100)
assert $ A.sortBy (comparing fst) s1 == s1

log "sortWith should reorder a list into ascending order based on the result of compare over a projection"
assert $ A.sortWith identity [1, 3, 2, 5, 6, 4] == [1, 2, 3, 4, 5, 6]

log "sortWith should not reorder elements that are equal according to a projection"
let s2 = map (Tuple "a") (A.range 1 100)
assert $ A.sortWith fst s2 == s2

log "take should keep the specified number of items from the front of an array, discarding the rest"
assert $ (A.take 1 [1, 2, 3]) == [1]
assert $ (A.take 2 [1, 2, 3]) == [1, 2]
Expand Down
14 changes: 14 additions & 0 deletions test/Test/Data/Array/ST.purs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ module Test.Data.Array.ST (testArrayST) where
import Prelude

import Control.Monad.ST as ST
import Data.Array (range)
import Data.Array.ST (withArray)
import Data.Array.ST as STA
import Data.Foldable (all)
import Data.Maybe (Maybe(..), isNothing)
import Data.Tuple (Tuple(..), fst)
import Effect (Effect)
import Effect.Console (log)
import Test.Assert (assert)
Expand Down Expand Up @@ -234,11 +236,23 @@ testArrayST = do
STA.sortBy (flip compare) =<< STA.unsafeThaw [1, 3, 2, 5, 6, 4]
) == [6, 5, 4, 3, 2, 1]

log "sortBy should not reorder elements that are equal according to a comparison function"
let s1 = map (Tuple "a") (range 1 100)
assert $ STA.run (
STA.sortBy (comparing fst) =<< STA.unsafeThaw s1
) == s1

log "sortWith should reorder a list into ascending order based on the result of compare over a projection"
assert $ STA.run (
STA.sortWith identity =<< STA.unsafeThaw [1, 3, 2, 5, 6, 4]
) == [1, 2, 3, 4, 5, 6]

log "sortWith should not reorder elements that are equal according to a projection"
let s2 = map (Tuple "a") (range 1 100)
assert $ STA.run (
STA.sortWith fst =<< STA.unsafeThaw s2
) == s2

log "splice should be able to delete multiple items at a specified index"

assert $ STA.run (do
Expand Down