From d49a1973e54f4277d794185011dad74d3b5d56c3 Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Tue, 28 Jul 2015 18:36:38 +0300 Subject: [PATCH] Change null handling in compareValuesBy(a, b, functions). Provide nullsFirst() and nullsLast() to extend Comparator to Comparator. --- libraries/stdlib/src/kotlin/util/Ordering.kt | 57 +++++++++++++++++-- libraries/stdlib/test/OrderingTest.kt | 19 +++++-- .../stdlib/test/collections/ArraysTest.kt | 6 +- .../stdlib/test/collections/CollectionTest.kt | 9 ++- 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/libraries/stdlib/src/kotlin/util/Ordering.kt b/libraries/stdlib/src/kotlin/util/Ordering.kt index 586e5cd28fa..7988a44cef9 100644 --- a/libraries/stdlib/src/kotlin/util/Ordering.kt +++ b/libraries/stdlib/src/kotlin/util/Ordering.kt @@ -17,6 +17,7 @@ package kotlin import java.util.Comparator +import kotlin.platform.platformName /** * Compares two values using the specified sequence of functions to calculate the result of the comparison. @@ -24,6 +25,7 @@ import java.util.Comparator * objects. As soon as the [Comparable] instances returned by a function for [a] and [b] values do not * compare as equal, the result of that comparison is returned. */ +deprecated("Use selector functions accepting nullable T as a receiver.") public fun compareValuesBy(a: T?, b: T?, vararg functions: (T) -> Comparable<*>?): Int { require(functions.size() > 0) if (a === b) return 0 @@ -38,15 +40,31 @@ public fun compareValuesBy(a: T?, b: T?, vararg functions: (T) -> Comp return 0 } +/** + * Compares two values using the specified sequence of functions to calculate the result of the comparison. + * The functions are called sequentially, receive the given values [a] and [b] and return [Comparable] + * objects. As soon as the [Comparable] instances returned by a function for [a] and [b] values do not + * compare as equal, the result of that comparison is returned. + */ +// TODO: Remove platformName after M13 +platformName("compareValuesByNullable") +public fun compareValuesBy(a: T, b: T, vararg selectors: (T) -> Comparable<*>?): Int { + require(selectors.size() > 0) + for (fn in selectors) { + val v1 = fn(a) + val v2 = fn(b) + val diff = compareValues(v1, v2) + if (diff != 0) return diff + } + return 0 +} + /** * Compares two values using the specified [selector] function to calculate the result of the comparison. * The function is applied to the given values [a] and [b] and return [Comparable] objects. * The result of comparison of these [Comparable] instances is returned. */ -public inline fun compareValuesBy(a: T?, b: T?, selector: (T) -> Comparable<*>?): Int { - if (a === b) return 0 - if (a == null) return -1 - if (b == null) return 1 +public inline fun compareValuesBy(a: T, b: T, selector: (T) -> Comparable<*>?): Int { return compareValues(selector(a), selector(b)) } @@ -150,4 +168,35 @@ inline public fun Comparator.thenComparator(inlineOptions(InlineOption.ON return if (previousCompare != 0) previousCompare else comparison(a, b) } } +} + +// Not so useful without type inference for receiver of expression +/** + * Extends the given comparator of non-nullable values to a comparator of nullable values + * considering null value less than any other value. + */ +public fun Comparator.nullsFirst(): Comparator { + return object: Comparator { + override fun compare(a: T?, b: T?): Int { + if (a === b) return 0 + if (a == null) return -1 + if (b == null) return 1 + return this@nullsFirst.compare(a, b) + } + } +} + +/** + * Extends the given comparator of non-nullable values to a comparator of nullable values + * considering null value greater than any other value. + */ +public fun Comparator.nullsLast(): Comparator { + return object: Comparator { + override fun compare(a: T?, b: T?): Int { + if (a === b) return 0 + if (a == null) return 1 + if (b == null) return -1 + return this@nullsLast.compare(a, b) + } + } } \ No newline at end of file diff --git a/libraries/stdlib/test/OrderingTest.kt b/libraries/stdlib/test/OrderingTest.kt index bfa0756ff9c..fea64583c92 100644 --- a/libraries/stdlib/test/OrderingTest.kt +++ b/libraries/stdlib/test/OrderingTest.kt @@ -34,12 +34,21 @@ class OrderingTest { assertTrue(diff == 0) } + Test fun compareNullables() { + val v1: Item? = this.v1 + val v2: Item? = null + val diff = compareValuesBy(v1, v2) { it?.rating } + assertTrue(diff > 0) + val diff2 = compareBy { it.rating }.thenBy { it.name }.nullsLast().compare(v1, v2) + assertTrue(diff2 < 0) + } + Test fun sortComparatorThenComparator() { val comparator = comparator { a, b -> a.name.compareTo(b.name) } thenComparator { a, b -> a.rating.compareTo(b.rating) } val diff = comparator.compare(v1, v2) assertTrue(diff > 0) - val items = arrayListOf(v1, v2).sortBy(comparator) + val items = arrayListOf(v1, v2).sortedWith(comparator) assertEquals(v2, items[0]) assertEquals(v1, items[1]) } @@ -49,7 +58,7 @@ class OrderingTest { val diff = comparator.compare(v1, v2) assertTrue(diff < 0) - val items = arrayListOf(v1, v2).sortBy(comparator) + val items = arrayListOf(v1, v2).sortedWith(comparator) assertEquals(v1, items[0]) assertEquals(v2, items[1]) } @@ -59,7 +68,7 @@ class OrderingTest { val diff = comparator.compare(v1, v2) assertTrue(diff < 0) - val items = arrayListOf(v1, v2).sortBy(comparator) + val items = arrayListOf(v1, v2).sortedWith(comparator) assertEquals(v1, items[0]) assertEquals(v2, items[1]) } @@ -68,7 +77,7 @@ class OrderingTest { val comparator = compareBy({ it.name }, { it.rating }) val diff = comparator.compare(v1, v2) assertTrue(diff > 0) - val items = arrayListOf(v1, v2).sortBy(comparator) + val items = arrayListOf(v1, v2).sortedWith(comparator) assertEquals(v2, items[0]) assertEquals(v1, items[1]) } @@ -85,7 +94,7 @@ class OrderingTest { } val diff = comparator.compare(v1, v2) assertTrue(diff > 0) - val items = arrayListOf(v1, v2).sortBy(comparator) + val items = arrayListOf(v1, v2).sortedWith(comparator) assertEquals(v2, items[0]) assertEquals(v1, items[1]) } diff --git a/libraries/stdlib/test/collections/ArraysTest.kt b/libraries/stdlib/test/collections/ArraysTest.kt index e3d321b856f..c5bfdf5ea58 100644 --- a/libraries/stdlib/test/collections/ArraysTest.kt +++ b/libraries/stdlib/test/collections/ArraysTest.kt @@ -827,9 +827,9 @@ class ArraysTest { test fun sortedNullableBy() { fun String.nullIfEmpty() = if (isEmpty()) null else this arrayOf(null, "").let { - expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}} - expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }} - expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }} + expect(listOf(null, "")) { it.sortedWith(compareBy { it }.nullsFirst()) } + expect(listOf("", null)) { it.sortedWith(compareByDescending { it }.nullsLast()) } + expect(listOf("", null)) { it.sortedWith(compareByDescending { it.nullIfEmpty() }.nullsLast() ) } } } diff --git a/libraries/stdlib/test/collections/CollectionTest.kt b/libraries/stdlib/test/collections/CollectionTest.kt index 5e2d47129ac..70f08c6902d 100644 --- a/libraries/stdlib/test/collections/CollectionTest.kt +++ b/libraries/stdlib/test/collections/CollectionTest.kt @@ -573,9 +573,12 @@ class CollectionTest { test fun sortedNullableBy() { fun String.nullIfEmpty() = if (isEmpty()) null else this listOf(null, "").let { - expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}} - expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }} - expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }} +// expect(listOf(null, "")) { it.sortedBy { it.orEmpty()}} +// expect(listOf("", null)) { it.sortedByDescending { it.orEmpty() }} +// expect(listOf("", null)) { it.sortedByDescending { it?.nullIfEmpty() }} + expect(listOf(null, "")) { it.sortedWith(compareBy { it }.nullsFirst()) } + expect(listOf("", null)) { it.sortedWith(compareByDescending { it }.nullsLast()) } + expect(listOf("", null)) { it.sortedWith(compareByDescending { it.nullIfEmpty() }.nullsLast() ) } } }