From 12cd4352fe471101889c2975b0d1b9c4a94c88a9 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 7 Aug 2020 12:40:42 +0100 Subject: [PATCH 1/5] Order for Ior --- core/src/main/scala/cats/data/Ior.scala | 23 ++++++++++++++++--- .../src/test/scala/cats/tests/IorSuite.scala | 4 +++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/cats/data/Ior.scala b/core/src/main/scala/cats/data/Ior.scala index 51d29772ae..cf21c1ed18 100644 --- a/core/src/main/scala/cats/data/Ior.scala +++ b/core/src/main/scala/cats/data/Ior.scala @@ -752,9 +752,20 @@ sealed abstract private[data] class IorInstances extends IorInstances0 { } } - implicit def catsDataEqForIor[A: Eq, B: Eq]: Eq[A Ior B] = - new Eq[A Ior B] { - def eqv(x: A Ior B, y: A Ior B): Boolean = x === y + implicit def catsDataEqForIor[A: Order, B: Order]: Order[A Ior B] = + new Order[A Ior B] { + + def compare(x: Ior[A, B], y: Ior[A, B]): Int = + (x, y) match { + case (Ior.Left(a1), Ior.Left(a2)) => Order[A].compare(a1, a2) + case (Ior.Left(_), _) => -1 + case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => Order[(A, B)].compare((a1, b1), (a2, b2)) + case (Ior.Both(_, _), Ior.Left(_)) => 1 + case (Ior.Both(_, _), Ior.Right(_)) => -1 + case (Ior.Right(b1), Ior.Right(b2)) => Order[B].compare(b1, b2) + case (Ior.Right(_), _) => 1 + } + } implicit def catsDataShowForIor[A: Show, B: Show]: Show[A Ior B] = @@ -879,6 +890,12 @@ sealed abstract private[data] class IorInstances0 { override def map[B, C](fa: A Ior B)(f: B => C): A Ior C = fa.map(f) } + + implicit def catsDataEqForIor[A: Eq, B: Eq]: Eq[A Ior B] = + new Eq[A Ior B] { + + def eqv(x: A Ior B, y: A Ior B): Boolean = x === y + } } sealed private[data] trait IorFunctions { diff --git a/tests/src/test/scala/cats/tests/IorSuite.scala b/tests/src/test/scala/cats/tests/IorSuite.scala index 35af32e1cc..ddbd573bca 100644 --- a/tests/src/test/scala/cats/tests/IorSuite.scala +++ b/tests/src/test/scala/cats/tests/IorSuite.scala @@ -3,7 +3,7 @@ package cats.tests import cats.{Bitraverse, MonadError, Semigroupal, Show, Traverse} import cats.data.{EitherT, Ior, NonEmptyChain, NonEmptyList, NonEmptySet} import cats.kernel.{Eq, Semigroup} -import cats.kernel.laws.discipline.SemigroupTests +import cats.kernel.laws.discipline.{OrderTests, SemigroupTests} import cats.laws.discipline.{ BifunctorTests, BitraverseTests, @@ -37,6 +37,8 @@ class IorSuite extends CatsSuite { checkAll("BitraverseTests Ior[*, *]", BitraverseTests[Ior].bitraverse[Option, Int, Int, Int, String, String, String]) checkAll("Bitraverse[Ior]", SerializableTests.serializable(Bitraverse[Ior])) + checkAll("Order[Ior[A: Order, B: Order]]", OrderTests[Ior[Int, Int]].order) + checkAll("Semigroup[Ior[A: Semigroup, B: Semigroup]]", SemigroupTests[Ior[List[Int], List[Int]]].semigroup) checkAll("SerializableTest Semigroup[Ior[A: Semigroup, B: Semigroup]]", SerializableTests.serializable(Semigroup[Ior[List[Int], List[Int]]]) From c300a9175064c496b12d3dca4271d025bdb45360 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 7 Aug 2020 12:59:15 +0100 Subject: [PATCH 2/5] Move compare definition onto Ior itself --- core/src/main/scala/cats/data/Ior.scala | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/core/src/main/scala/cats/data/Ior.scala b/core/src/main/scala/cats/data/Ior.scala index cf21c1ed18..3e3c3b01c0 100644 --- a/core/src/main/scala/cats/data/Ior.scala +++ b/core/src/main/scala/cats/data/Ior.scala @@ -708,6 +708,17 @@ sealed abstract class Ior[+A, +B] extends Product with Serializable { (a, b) => that.fold(a2 => false, b2 => false, (a2, b2) => AA.eqv(a, a2) && BB.eqv(b, b2)) ) + final def compare[AA >: A, BB >: B](that: AA Ior BB)(implicit AA: Order[AA], BB: Order[BB]): Int = + (this, that) match { + case (Ior.Left(a1), Ior.Left(a2)) => AA.compare(a1, a2) + case (Ior.Left(_), _) => -1 + case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => Order[(AA, BB)].compare((a1, b1), (a2, b2)) + case (Ior.Both(_, _), Ior.Left(_)) => 1 + case (Ior.Both(_, _), Ior.Right(_)) => -1 + case (Ior.Right(b1), Ior.Right(b2)) => BB.compare(b1, b2) + case (Ior.Right(_), _) => 1 + } + final def show[AA >: A, BB >: B](implicit AA: Show[AA], BB: Show[BB]): String = fold( a => s"Ior.Left(${AA.show(a)})", @@ -755,17 +766,7 @@ sealed abstract private[data] class IorInstances extends IorInstances0 { implicit def catsDataEqForIor[A: Order, B: Order]: Order[A Ior B] = new Order[A Ior B] { - def compare(x: Ior[A, B], y: Ior[A, B]): Int = - (x, y) match { - case (Ior.Left(a1), Ior.Left(a2)) => Order[A].compare(a1, a2) - case (Ior.Left(_), _) => -1 - case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => Order[(A, B)].compare((a1, b1), (a2, b2)) - case (Ior.Both(_, _), Ior.Left(_)) => 1 - case (Ior.Both(_, _), Ior.Right(_)) => -1 - case (Ior.Right(b1), Ior.Right(b2)) => Order[B].compare(b1, b2) - case (Ior.Right(_), _) => 1 - } - + def compare(x: Ior[A, B], y: Ior[A, B]): Int = x.compare(y) } implicit def catsDataShowForIor[A: Show, B: Show]: Show[A Ior B] = From 926a6db74b32e2b67d6909d2071ff4df8c8a4345 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 7 Aug 2020 13:58:17 +0100 Subject: [PATCH 3/5] Fix typo --- core/src/main/scala/cats/data/Ior.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/data/Ior.scala b/core/src/main/scala/cats/data/Ior.scala index 3e3c3b01c0..3526a31df7 100644 --- a/core/src/main/scala/cats/data/Ior.scala +++ b/core/src/main/scala/cats/data/Ior.scala @@ -763,7 +763,7 @@ sealed abstract private[data] class IorInstances extends IorInstances0 { } } - implicit def catsDataEqForIor[A: Order, B: Order]: Order[A Ior B] = + implicit def catsDataOrderForIor[A: Order, B: Order]: Order[A Ior B] = new Order[A Ior B] { def compare(x: Ior[A, B], y: Ior[A, B]): Int = x.compare(y) From 218246285b4445b66e83685ceb747380f4fef784 Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 7 Aug 2020 17:03:17 +0100 Subject: [PATCH 4/5] Reduce allocations in compare --- core/src/main/scala/cats/data/Ior.scala | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/core/src/main/scala/cats/data/Ior.scala b/core/src/main/scala/cats/data/Ior.scala index 3526a31df7..0a977c8925 100644 --- a/core/src/main/scala/cats/data/Ior.scala +++ b/core/src/main/scala/cats/data/Ior.scala @@ -710,13 +710,16 @@ sealed abstract class Ior[+A, +B] extends Product with Serializable { final def compare[AA >: A, BB >: B](that: AA Ior BB)(implicit AA: Order[AA], BB: Order[BB]): Int = (this, that) match { - case (Ior.Left(a1), Ior.Left(a2)) => AA.compare(a1, a2) - case (Ior.Left(_), _) => -1 - case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => Order[(AA, BB)].compare((a1, b1), (a2, b2)) - case (Ior.Both(_, _), Ior.Left(_)) => 1 - case (Ior.Both(_, _), Ior.Right(_)) => -1 - case (Ior.Right(b1), Ior.Right(b2)) => BB.compare(b1, b2) - case (Ior.Right(_), _) => 1 + case (Ior.Left(a1), Ior.Left(a2)) => AA.compare(a1, a2) + case (Ior.Left(_), _) => -1 + case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => { + val r = AA.compare(a1, a2) + if (r == 0) BB.compare(b1, b2) else r + } + case (Ior.Both(_, _), Ior.Left(_)) => 1 + case (Ior.Both(_, _), Ior.Right(_)) => -1 + case (Ior.Right(b1), Ior.Right(b2)) => BB.compare(b1, b2) + case (Ior.Right(_), _) => 1 } final def show[AA >: A, BB >: B](implicit AA: Show[AA], BB: Show[BB]): String = From 64de92f8ba1dfb05cb5064c7f8a46b0d77acd74c Mon Sep 17 00:00:00 2001 From: Tim Spence Date: Fri, 7 Aug 2020 20:30:38 +0100 Subject: [PATCH 5/5] Make Both > Right --- core/src/main/scala/cats/data/Ior.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/cats/data/Ior.scala b/core/src/main/scala/cats/data/Ior.scala index 0a977c8925..c11078e39c 100644 --- a/core/src/main/scala/cats/data/Ior.scala +++ b/core/src/main/scala/cats/data/Ior.scala @@ -710,16 +710,16 @@ sealed abstract class Ior[+A, +B] extends Product with Serializable { final def compare[AA >: A, BB >: B](that: AA Ior BB)(implicit AA: Order[AA], BB: Order[BB]): Int = (this, that) match { - case (Ior.Left(a1), Ior.Left(a2)) => AA.compare(a1, a2) - case (Ior.Left(_), _) => -1 + case (Ior.Left(a1), Ior.Left(a2)) => AA.compare(a1, a2) + case (Ior.Left(_), _) => -1 + case (Ior.Right(b1), Ior.Right(b2)) => BB.compare(b1, b2) + case (Ior.Right(_), Ior.Left(_)) => 1 + case (Ior.Right(_), Ior.Both(_, _)) => -1 case (Ior.Both(a1, b1), Ior.Both(a2, b2)) => { val r = AA.compare(a1, a2) if (r == 0) BB.compare(b1, b2) else r } - case (Ior.Both(_, _), Ior.Left(_)) => 1 - case (Ior.Both(_, _), Ior.Right(_)) => -1 - case (Ior.Right(b1), Ior.Right(b2)) => BB.compare(b1, b2) - case (Ior.Right(_), _) => 1 + case (Ior.Both(_, _), _) => 1 } final def show[AA >: A, BB >: B](implicit AA: Show[AA], BB: Show[BB]): String =