Skip to content

[Belt] Add List.sort #2666

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.

Sign up for GitHub

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 5 commits into from
Mar 28, 2018
Merged

[Belt] Add List.sort #2666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b45bfe8
[Belt] Add List.sort
zploskey Mar 21, 2018
e94d8c0
[Belt] Add List.sortU
zploskey Mar 24, 2018
05e516d
Fix dependency on SortArray
zploskey Mar 24, 2018
228d0db
Remove unnecessary begin/end
zploskey Mar 26, 2018
b978c47
Make sort wrap sortU
zploskey Mar 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jscomp/others/.depend
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ belt_internalAVLset.cmj : belt_SortArray.cmj belt_Id.cmj belt_Array.cmj \
belt_internalAVLset.cmi
belt_internalAVLtree.cmj : belt_SortArray.cmj belt_Id.cmj belt_Array.cmj \
belt_internalAVLtree.cmi
belt_List.cmj : belt_Array.cmj belt_List.cmi
belt_List.cmj : belt_Array.cmj belt_SortArray.cmj belt_List.cmi
belt_SortArray.cmj : belt_SortArrayString.cmj belt_SortArrayInt.cmj \
belt_Array.cmj belt_SortArray.cmi
belt_SortArrayInt.cmj : belt_Array.cmj belt_SortArrayInt.cmi
Expand Down
7 changes: 7 additions & 0 deletions jscomp/others/belt_List.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,13 @@ let setAssocU xs x k eq =

let setAssoc xs x k eq = setAssocU xs x k (fun [@bs] a b -> eq a b)

let sortU xs cmp =
let arr = toArray xs in
Belt_SortArray.stableSortInPlaceByU arr cmp;
fromArray arr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, in belt, we should define sortU first and wrap sort like this

let sort xs cmp = sortU xs (fun[@bs] x y -> cmp x y)

The thing is in your sort, you are calling stableSortInPlaceBy instead of stableSortInPlaceByU which is slower

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks!

let sort xs cmp = sortU xs (fun [@bs] x y -> cmp x y)

let rec getByU xs p =
match xs with
| [] -> None
Expand Down
12 changes: 9 additions & 3 deletions jscomp/others/belt_List.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,14 @@ val setAssoc: ('a * 'c) t -> 'a -> 'c -> ('a -> 'a -> bool) -> ('a * 'c) t
setAssoc [1,"a"; 3, "c"] 2 "2" (=) =
[2,"2"; 1,"a"; 3, "c"]
]}
*)


*)


val sortU: 'a t -> ('a -> 'a -> int [@bs]) -> 'a t
val sort: 'a t -> ('a -> 'a -> int) -> 'a t
(** [sort xs]
Returns a sorted list.
@example {[
sort (fun a b -> a - b) [5; 4; 9; 3; 7] = [3; 4; 5; 7; 9]
]}
*)
76 changes: 69 additions & 7 deletions jscomp/test/bs_list_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2181,7 +2181,69 @@ makeTest(2);

makeTest(3);

b("File \"bs_list_test.ml\", line 320, characters 4-11", 1 - Belt_List.eq(/* :: */[
function cmp(a, b) {
return a - b | 0;
}

eq("SORT", Belt_List.sort(/* :: */[
5,
/* :: */[
4,
/* :: */[
3,
/* :: */[
2,
/* [] */0
]
]
]
], cmp), /* :: */[
2,
/* :: */[
3,
/* :: */[
4,
/* :: */[
5,
/* [] */0
]
]
]
]);

eq("SORT", Belt_List.sort(/* :: */[
3,
/* :: */[
9,
/* :: */[
37,
/* :: */[
3,
/* :: */[
1,
/* [] */0
]
]
]
]
], cmp), /* :: */[
1,
/* :: */[
3,
/* :: */[
3,
/* :: */[
9,
/* :: */[
37,
/* [] */0
]
]
]
]
]);

b("File \"bs_list_test.ml\", line 326, characters 4-11", 1 - Belt_List.eq(/* :: */[
1,
/* :: */[
2,
Expand All @@ -2200,7 +2262,7 @@ b("File \"bs_list_test.ml\", line 320, characters 4-11", 1 - Belt_List.eq(/* ::
return +(x === y);
})));

b("File \"bs_list_test.ml\", line 321, characters 4-11", Belt_List.eq(/* :: */[
b("File \"bs_list_test.ml\", line 327, characters 4-11", Belt_List.eq(/* :: */[
1,
/* :: */[
2,
Expand All @@ -2222,7 +2284,7 @@ b("File \"bs_list_test.ml\", line 321, characters 4-11", Belt_List.eq(/* :: */[
return +(x === y);
})));

b("File \"bs_list_test.ml\", line 322, characters 4-11", 1 - Belt_List.eq(/* :: */[
b("File \"bs_list_test.ml\", line 328, characters 4-11", 1 - Belt_List.eq(/* :: */[
1,
/* :: */[
2,
Expand All @@ -2244,7 +2306,7 @@ b("File \"bs_list_test.ml\", line 322, characters 4-11", 1 - Belt_List.eq(/* ::
return +(x === y);
})));

b("File \"bs_list_test.ml\", line 323, characters 4-11", 1 - Belt_List.eq(/* :: */[
b("File \"bs_list_test.ml\", line 329, characters 4-11", 1 - Belt_List.eq(/* :: */[
1,
/* :: */[
2,
Expand Down Expand Up @@ -2279,7 +2341,7 @@ var u1 = Belt_List.keepMap(u0, (function (x) {
}
}));

eq("File \"bs_list_test.ml\", line 327, characters 5-12", u1, /* :: */[
eq("File \"bs_list_test.ml\", line 333, characters 5-12", u1, /* :: */[
1,
/* :: */[
8,
Expand All @@ -2290,7 +2352,7 @@ eq("File \"bs_list_test.ml\", line 327, characters 5-12", u1, /* :: */[
]
]);

b("File \"bs_list_test.ml\", line 328, characters 4-11", Caml_obj.caml_equal(Belt_List.keepMap(/* :: */[
b("File \"bs_list_test.ml\", line 334, characters 4-11", Caml_obj.caml_equal(Belt_List.keepMap(/* :: */[
1,
/* :: */[
2,
Expand All @@ -2316,7 +2378,7 @@ b("File \"bs_list_test.ml\", line 328, characters 4-11", Caml_obj.caml_equal(Bel
]
]));

b("File \"bs_list_test.ml\", line 332, characters 4-11", +(Belt_List.keepMap(/* :: */[
b("File \"bs_list_test.ml\", line 338, characters 4-11", +(Belt_List.keepMap(/* :: */[
1,
/* :: */[
2,
Expand Down
6 changes: 6 additions & 0 deletions jscomp/test/bs_list_test.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ let () =
makeTest 2;
makeTest 3

let () =
let (=~) = eq "SORT" in
let cmp a b = a - b in
N.sort [5; 4; 3; 2] cmp =~ [2; 3; 4; 5];
N.sort [3; 9; 37; 3; 1] cmp =~ [1; 3; 3; 9; 37]

let () =
b __LOC__ (not @@ N.eq [1;2;3] [1;2] (fun x y -> x = y));
b __LOC__ (N.eq [1;2;3] [1;2;3] (fun x y -> x = y));
Expand Down
13 changes: 13 additions & 0 deletions lib/js/belt_List.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var Curry = require("./curry.js");
var Belt_Array = require("./belt_Array.js");
var Belt_SortArray = require("./belt_SortArray.js");

function head(x) {
if (x) {
Expand Down Expand Up @@ -1209,6 +1210,16 @@ function setAssoc(xs, x, k, eq) {
return setAssocU(xs, x, k, Curry.__2(eq));
}

function sortU(xs, cmp) {
var arr = toArray(xs);
Belt_SortArray.stableSortInPlaceByU(arr, cmp);
return fromArray(arr);
}

function sort(xs, cmp) {
return sortU(xs, Curry.__2(cmp));
}

function getByU(_xs, p) {
while(true) {
var xs = _xs;
Expand Down Expand Up @@ -1445,4 +1456,6 @@ exports.removeAssocU = removeAssocU;
exports.removeAssoc = removeAssoc;
exports.setAssocU = setAssocU;
exports.setAssoc = setAssoc;
exports.sortU = sortU;
exports.sort = sort;
/* No side effect */