From c8393fdf44862cf09fca6ef4bc7899e7c0386f79 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 16 Jan 2015 14:27:46 +1000 Subject: SI-9087 Fix min/max of reversed Double/Float orderings As diagnosed by the reporter, we needed additional overrides due to the way these orderings are implemented. I've added tests to show other methods and other orderings are working correctly. After writing that, I found a scalacheck test related to NaN handling that also covers `Ordering`. I had to correct the assertion in the tests of `reverse.{min,max}`. --- src/library/scala/math/Ordering.scala | 5 +++ test/files/scalacheck/nan-ordering.scala | 16 ++++----- test/junit/scala/math/OrderingTest.scala | 61 ++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 test/junit/scala/math/OrderingTest.scala diff --git a/src/library/scala/math/Ordering.scala b/src/library/scala/math/Ordering.scala index 0d7ea8bce2..827cccc77e 100644 --- a/src/library/scala/math/Ordering.scala +++ b/src/library/scala/math/Ordering.scala @@ -284,6 +284,9 @@ object Ordering extends LowPriorityOrderingImplicits { override def gteq(x: Float, y: Float): Boolean = outer.gteq(y, x) override def lt(x: Float, y: Float): Boolean = outer.lt(y, x) override def gt(x: Float, y: Float): Boolean = outer.gt(y, x) + override def min(x: Float, y: Float): Float = outer.max(x, y) + override def max(x: Float, y: Float): Float = outer.min(x, y) + } } implicit object Float extends FloatOrdering @@ -309,6 +312,8 @@ object Ordering extends LowPriorityOrderingImplicits { override def gteq(x: Double, y: Double): Boolean = outer.gteq(y, x) override def lt(x: Double, y: Double): Boolean = outer.lt(y, x) override def gt(x: Double, y: Double): Boolean = outer.gt(y, x) + override def min(x: Double, y: Double): Double = outer.max(x, y) + override def max(x: Double, y: Double): Double = outer.min(x, y) } } implicit object Double extends DoubleOrdering diff --git a/test/files/scalacheck/nan-ordering.scala b/test/files/scalacheck/nan-ordering.scala index 2094a46e37..05e97a13c9 100644 --- a/test/files/scalacheck/nan-ordering.scala +++ b/test/files/scalacheck/nan-ordering.scala @@ -42,16 +42,16 @@ object Test extends Properties("NaN-Ordering") { property("Float equiv") = forAll(specFloats, specFloats) { (d1, d2) => numFloat.equiv(d1, d2) == (d1 == d2) } property("Float reverse.min") = forAll(specFloats, specFloats) { (d1, d2) => { - val mathmin = math.min(d1, d2) + val mathmax = math.max(d1, d2) val numericmin = numFloat.reverse.min(d1, d2) - mathmin == numericmin || mathmin.isNaN && numericmin.isNaN + mathmax == numericmin || mathmax.isNaN && numericmin.isNaN } } property("Float reverse.max") = forAll(specFloats, specFloats) { (d1, d2) => { - val mathmax = math.max(d1, d2) + val mathmin = math.min(d1, d2) val numericmax = numFloat.reverse.max(d1, d2) - mathmax == numericmax || mathmax.isNaN && numericmax.isNaN + mathmin == numericmax || mathmin.isNaN && numericmax.isNaN } } @@ -105,16 +105,16 @@ object Test extends Properties("NaN-Ordering") { property("Double equiv") = forAll(specDoubles, specDoubles) { (d1, d2) => numDouble.equiv(d1, d2) == (d1 == d2) } property("Double reverse.min") = forAll(specDoubles, specDoubles) { (d1, d2) => { - val mathmin = math.min(d1, d2) + val mathmax = math.max(d1, d2) val numericmin = numDouble.reverse.min(d1, d2) - mathmin == numericmin || mathmin.isNaN && numericmin.isNaN + mathmax == numericmin || mathmax.isNaN && numericmin.isNaN } } property("Double reverse.max") = forAll(specDoubles, specDoubles) { (d1, d2) => { - val mathmax = math.max(d1, d2) + val mathmin = math.min(d1, d2) val numericmax = numDouble.reverse.max(d1, d2) - mathmax == numericmax || mathmax.isNaN && numericmax.isNaN + mathmin == numericmax || mathmin.isNaN && numericmax.isNaN } } diff --git a/test/junit/scala/math/OrderingTest.scala b/test/junit/scala/math/OrderingTest.scala new file mode 100644 index 0000000000..218622b8b4 --- /dev/null +++ b/test/junit/scala/math/OrderingTest.scala @@ -0,0 +1,61 @@ +package scala.math + +import org.junit.Assert._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(classOf[JUnit4]) +class OrderingTest { + + /* Test for SI-9077 */ + @Test + def testReverseOrdering { + def check[T: Ordering](t1: T, t2: T): Unit = { + val O = Ordering[T] + val R = O.reverse + assertEquals(O.min(t1, t2), R.max(t1, t2)) + assertEquals(O.max(t1, t2), R.min(t1, t2)) + + assertEquals(O.lteq(t1, t2), R.lteq(t2, t1)) + assertEquals(O.lt(t1, t2), R.lt(t2, t1)) + assertEquals(O.gteq(t1, t2), R.gteq(t2, t1)) + assertEquals(O.gt(t1, t2), R.gt(t2, t1)) + assertEquals(O.compare(t1, t2), R.compare(t2, t1)) + + assertEquals(O.equiv(t1, t2), R.equiv(t1, t2)) + + assertEquals(O.on((x: T) => x).min(t1, t2), R.on((x: T) => x).max(t1, t2)) + + assertEquals(O.tryCompare(t1, t2), R.tryCompare(t2, t1)) + + assertEquals(O.mkOrderingOps(t1).<(t2), R.mkOrderingOps(t2).<(t1)) + assertEquals(O.mkOrderingOps(t1).<=(t2), R.mkOrderingOps(t2).<=(t1)) + assertEquals(O.mkOrderingOps(t1).>(t2), R.mkOrderingOps(t2).>(t1)) + assertEquals(O.mkOrderingOps(t1).>=(t2), R.mkOrderingOps(t2).>=(t1)) + + assertEquals(O.mkOrderingOps(t1).min(t2), R.mkOrderingOps(t1).max(t2)) + assertEquals(O.mkOrderingOps(t1).max(t2), R.mkOrderingOps(t1).min(t2)) + } + def checkAll[T: Ordering](ts: T*): Unit = { + for (t1 <- ts; t2 <- ts) check(t1, t2) + } + checkAll[Unit](()) + checkAll[Boolean](true, false) + checkAll[Byte](Byte.MinValue, -1.toByte, 0.toByte, 1.toByte, Byte.MaxValue) + checkAll[Char](Char.MinValue, -1.toChar, 0.toChar, 1.toChar, Char.MaxValue) + checkAll[Short](Short.MinValue, -1, 0, 1, Short.MaxValue) + checkAll[Int](Int.MinValue, -1, 0, 1, Int.MaxValue) + checkAll[Double](Double.MinValue, -1, -0, 0, 1, Double.MaxValue) + checkAll[Float](Float.MinValue, -1, -0, 0, 1, Float.MaxValue) + + checkAll[BigInt](Int.MinValue, -1, 0, 1, Int.MaxValue) + checkAll[BigDecimal](Int.MinValue, -1, -0, 1, Int.MaxValue) + checkAll[String]("", "a", "b", "bb") + checkAll[String]("", "a", "b", "bb") + checkAll[Option[Int]](None, Some(1), Some(2)) + checkAll[Iterable[Int]](Nil, List(1), List(1, 2)) + checkAll[(Int, Int)]((1, 2), (1, 3), (4, 5)) + } +} + -- cgit v1.2.3