Use Chain For Arbitrary, don't deoptimize combineAll and syntax exten…

…ds AnyVal
typelevel · Aug 15, 2018 · 1160f77 · 1160f77
1 parent c5d3a07
commit 1160f77
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 15 deletions.
diff --git a/core/src/main/scala/cats/data/NonEmptyChain.scala b/core/src/main/scala/cats/data/NonEmptyChain.scala
@@ -70,7 +70,7 @@ private[data] object NonEmptyChainImpl extends NonEmptyChainInstances {
 }
 
 
-sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
+class NonEmptyChainOps[A](val value: NonEmptyChain[A]) extends AnyVal {
 
   /**
    * Converts this chain to a `Chain`
@@ -106,8 +106,8 @@ sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
    * {{{
    * scala> import cats.data.NonEmptyChain
    * scala> val nec = NonEmptyChain(1, 2, 4, 5)
-   * scala> nec ++ NonEmptyChain(1, 2, 7)
-   * res0: cats.data.NonEmptyChain[Int] = Chain(1, 2, 4, 5, 7)
+   * scala> nec ++ NonEmptyChain(7, 8)
+   * res0: cats.data.NonEmptyChain[Int] = Chain(1, 2, 4, 5, 7, 8)
    * }}}
    */
   final def concat[A2 >: A](c: NonEmptyChain[A2]): NonEmptyChain[A2] =
@@ -166,8 +166,7 @@ sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
    * Converts this chain to a `NonEmptyList`.
    * {{{
    * scala> import cats.data.NonEmptyChain
-   * scala> import cats.implicits._
-   * scala> val nec = NonEmptyChain.of(1, 2, 3, 4, 5)
+   * scala> val nec = NonEmptyChain(1, 2, 3, 4, 5)
    * scala> nec.toNonEmptyList
    * res0: cats.data.NonEmptyList[Int] = NonEmptyList(1, 2, 3, 4, 5)
    * }}}
@@ -178,9 +177,8 @@ sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
    * Converts this chain to a `NonEmptyVector`.
    * {{{
    * scala> import cats.data.NonEmptyChain
-   * scala> import cats.implicits._
-   * scala> val nec = NonEmptyChain.of(1, 2, 3, 4, 5)
-   * scala> nec.NonEmptyVector
+   * scala> val nec = NonEmptyChain(1, 2, 3, 4, 5)
+   * scala> nec.toNonEmptyVector
    * res0: cats.data.NonEmptyVector[Int] = NonEmptyVector(1, 2, 3, 4, 5)
    * }}}
    */
@@ -295,10 +293,7 @@ sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
    * Reduce using the Semigroup of A
    */
   final def reduce[AA >: A](implicit S: Semigroup[AA]): AA = {
-    val iter: Iterator[AA] = toChain.iterator
-    var result = iter.next
-    while (iter.hasNext) { result = S.combine(result, iter.next) }
-    result
+    S.combineAllOption(iterator).get
   }
 
   /**
@@ -343,7 +338,6 @@ sealed class NonEmptyChainOps[A](val value: NonEmptyChain[A]) {
    *
    * {{{
    * scala> import cats.data.NonEmptyChain
-   * scala> import cats.implicits._
    * scala> val as = NonEmptyChain(1, 2, 3)
    * scala> val bs = NonEmptyChain("A", "B", "C")
    * scala> as.zipWith(bs)(_ + _)

diff --git a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala
@@ -77,10 +77,15 @@ object arbitrary extends ArbitraryInstances0 {
     Cogen[List[A]].contramap(_.toList)
 
   implicit def catsLawsArbitraryForNonEmptyChain[A](implicit A: Arbitrary[A]): Arbitrary[NonEmptyChain[A]] =
-    Arbitrary(implicitly[Arbitrary[List[A]]].arbitrary.flatMap(fa => A.arbitrary.map(a => NonEmptyChain(a, fa: _*))))
+    Arbitrary(implicitly[Arbitrary[Chain[A]]].arbitrary.flatMap(fa =>
+      Gen.oneOf(
+        A.arbitrary.map(a => NonEmptyChain.fromChainPrepend(a, fa)),
+        A.arbitrary.map(a => NonEmptyChain.fromChainAppend(fa, a)),
+        A.arbitrary.map(NonEmptyChain.one)
+    )))
 
   implicit def catsLawsCogenForNonEmptyChain[A](implicit A: Cogen[A]): Cogen[NonEmptyChain[A]] =
-    Cogen[List[A]].contramap(_.toChain.toList)
+    Cogen[Chain[A]].contramap(_.toChain)
 
 
   implicit def catsLawsArbitraryForZipNonEmptyList[A](implicit A: Arbitrary[A]): Arbitrary[ZipNonEmptyList[A]] =