Skip to content

Commit 4e369f5

Browse files
jbgijbgi
committed
Deprecate unsafe Ord instance and List#groupBy that use them.
1 parent 726f024 commit 4e369f5

5 files changed

Lines changed: 24 additions & 10 deletions

File tree

‎core/src/main/java/fj/Ord.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -478,12 +478,14 @@ public static <A extends Comparable<A>> Ord<A> comparableOrd() {
478478

479479
/**
480480
* An order instance that uses {@link Object#hashCode()} for computing the order and equality,
481-
* thus objects returning the same hashCode are considered to be equals (check {@link #hashEqualsOrd()}
482-
* for an additional check on {@link Object#equals(Object)}).
481+
* thus objects returning the same hashCode are considered to be equals.
482+
* This is not safe and therefore this method is deprecated.
483483
*
484484
* @return An order instance that is based on {@link Object#hashCode()}.
485-
* @see #hashEqualsOrd()
485+
*
486+
* @deprecated As of release 4.7.
486487
*/
488+
@Deprecated
487489
public static <A> Ord<A> hashOrd() {
488490
return ord(a -> {
489491
int aHash = a.hashCode();
@@ -495,9 +497,13 @@ public static <A> Ord<A> hashOrd() {
495497
* An order instance that uses {@link Object#hashCode()} and {@link Object#equals} for computing
496498
* the order and equality. First the hashCode is compared, if this is equal, objects are compared
497499
* using {@link Object#equals}.
500+
* WARNING: This ordering violate antisymmetry on hash collisions.
498501
*
499502
* @return An order instance that is based on {@link Object#hashCode()} and {@link Object#equals}.
503+
*
504+
* @deprecated As of release 4.7.
500505
*/
506+
@Deprecated
501507
public static <A> Ord<A> hashEqualsOrd() {
502508
return ord(a -> {
503509
int aHash = a.hashCode();

‎core/src/main/java/fj/data/List.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,11 +1299,15 @@ public final A mode(final Ord<A> o) {
12991299

13001300
/**
13011301
* Groups the elements of this list by a given keyFunction into a {@link TreeMap}.
1302-
* The ordering of the keys is determined by {@link fj.Ord#hashOrd()}.
1302+
* The ordering of the keys is determined by {@link fj.Ord#hashOrd()} (ie. Object#hasCode).
1303+
* This is not safe and therefore this method is deprecated.
13031304
*
13041305
* @param keyFunction The function to select the keys for the map.
13051306
* @return A TreeMap containing the keys with the accumulated list of matched elements.
1307+
*
1308+
* @deprecated As of release 4.7, use {@link #groupBy(F, Ord)}
13061309
*/
1310+
@Deprecated
13071311
public final <B> TreeMap<B, List<A>> groupBy(final F<A, B> keyFunction) {
13081312
return groupBy(keyFunction, Ord.hashOrd());
13091313
}
@@ -1322,12 +1326,16 @@ public final <B> TreeMap<B, List<A>> groupBy(final F<A, B> keyFunction, final Or
13221326
/**
13231327
* Groups the elements of this list by a given keyFunction into a {@link TreeMap} and transforms
13241328
* the matching elements with the given valueFunction. The ordering of the keys is determined by
1325-
* {@link fj.Ord#hashOrd()}.
1329+
* {@link fj.Ord#hashOrd()} (ie. Object#hasCode).
1330+
* This is not safe and therefore this method is deprecated.
13261331
*
13271332
* @param keyFunction The function to select the keys for the map.
13281333
* @param valueFunction The function to apply on each matching value.
13291334
* @return A TreeMap containing the keys with the accumulated list of matched and mapped elements.
1335+
*
1336+
* @deprecated As of release 4.7, use {@link #groupBy(F, F, Ord)}
13301337
*/
1338+
@Deprecated
13311339
public final <B, C> TreeMap<B, List<C>> groupBy(
13321340
final F<A, B> keyFunction,
13331341
final F<A, C> valueFunction) {

‎demo/src/main/java/fj/demo/List_groupBy.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ public static void main(final String... args) {
1818
private static void keyDemo() {
1919
System.out.println("KeyDemo");
2020
final List<String> words = list("Hello", "World", "how", "are", "your", "doing");
21-
final TreeMap<Integer, List<String>> lengthMap = words.groupBy(String::length);
21+
final TreeMap<Integer, List<String>> lengthMap = words.groupBy(String::length, Ord.intOrd);
2222

2323
lengthMap.forEach(entry -> System.out.println(String.format("Words with %d chars: %s", entry._1(), entry._2())));
2424
}
2525

2626
private static void keyValueDemo() {
2727
System.out.println("KeyValueDemo");
2828
final List<Integer> xs = list(1, 2, 3, 4, 5, 6, 7, 8, 9);
29-
final TreeMap<Integer, List<String>> result = xs.groupBy(x -> x % 3, Integer::toBinaryString);
29+
final TreeMap<Integer, List<String>> result = xs.groupBy(x -> x % 3, Integer::toBinaryString, Ord.intOrd);
3030

3131
result.forEach(entry -> System.out.println(String.format("Numbers with reminder %d are %s", entry._1(), entry._2())));
3232
}
@@ -35,7 +35,7 @@ private static void keyValueAccDemo() {
3535
System.out.println("KeyValueAccDemo");
3636
final List<String> words = list("Hello", "World", "how", "are", "your", "doing");
3737
final TreeMap<Integer, Integer> lengthCounts =
38-
words.groupBy(String::length, Function.identity(), 0, (word, sum) -> sum + 1, Ord.hashOrd());
38+
words.groupBy(String::length, Function.identity(), 0, (word, sum) -> sum + 1, Ord.intOrd);
3939

4040
lengthCounts.forEach(entry -> System.out.println(String.format("Words with %d chars: %s", entry._1(), entry._2())));
4141
}

‎props-core-scalacheck/src/test/scala/fj/data/CheckList.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ object CheckList extends Properties("List") {
187187
join(a)))
188188

189189
property("groupBy") = forAll((a: List[Int]) => {
190-
val result = a.groupBy((x: Int) => (x % 2 == 0): lang.Boolean)
190+
val result = a.groupBy((x: Int) => (x % 2 == 0): lang.Boolean, Ord.booleanOrd)
191191
result.get(true).forall((xs: List[Int]) => xs.forall((x: Int) => (x % 2 == 0): lang.Boolean): lang.Boolean) &&
192192
result.get(false).forall((xs: List[Int]) => xs.forall((x: Int) => (x % 2 != 0): lang.Boolean): lang.Boolean) &&
193193
a.map((x: Int) => (x % 2) == 0: lang.Boolean).nub().length() == result.size()

‎props-core/src/test/java/fj/data/properties/ListProperties.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ public Property nub() {
255255

256256
public Property groupBy() {
257257
return property(arbList(arbInteger), list -> {
258-
final TreeMap<Boolean, List<Integer>> map = list.groupBy(i -> i % 2 == 0);
258+
final TreeMap<Boolean, List<Integer>> map = list.groupBy(i -> i % 2 == 0, Ord.booleanOrd);
259259
final List<Integer> list1 = map.get(true).orSome(nil());
260260
final List<Integer> list2 = map.get(false).orSome(nil());
261261
return prop(list.length() == list1.length() + list2.length())

0 commit comments

Comments
 (0)