Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foldable/Reducible intercalate update #1522

Merged
merged 2 commits into from
Jan 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions core/src/main/scala/cats/Foldable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,20 @@ import simulacrum.typeclass
* }}}
*/
def intercalate[A](fa: F[A], a: A)(implicit A: Monoid[A]): A =
reduceLeftOption(fa){ (acc, aa) =>
A.combine(acc, A.combine(a, aa))
}.getOrElse(A.empty)
A.combineAll(intersperseList(toList(fa), a))

protected def intersperseList[A](xs: List[A], x: A): List[A] = {
val bld = List.newBuilder[A]
val it = xs.iterator
if (it.hasNext) {
bld += it.next
while(it.hasNext) {
bld += x
bld += it.next
}
}
bld.result
}

def compose[G[_]: Foldable]: Foldable[λ[α => F[G[α]]]] =
new ComposedFoldable[F, G] {
Expand Down
9 changes: 5 additions & 4 deletions core/src/main/scala/cats/Reducible.scala
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,11 @@ import simulacrum.typeclass
* }}}
*/
def intercalate1[A](fa: F[A], a: A)(implicit A: Semigroup[A]): A =
reduceLeft(fa)((acc, aa) => A.combine(acc, A.combine(a, aa)))

override def intercalate[A](fa: F[A], a: A)(implicit A: Monoid[A]): A =
intercalate1(fa, a)
toNonEmptyList(fa) match {
case NonEmptyList(hd, Nil) => hd
case NonEmptyList(hd, tl) =>
Reducible[NonEmptyList].reduce(NonEmptyList(hd, a :: intersperseList(tl, a)))
}
}

/**
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/scala/cats/data/NonEmptyList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) {
def reduceLeft[AA >: A](f: (AA, AA) => AA): AA =
tail.foldLeft[AA](head)(f)

/**
* Reduce using the `Semigroup` of `AA`.
*/
def reduce[AA >: A](implicit S: Semigroup[AA]): AA =
S.combineAllOption(toList).get

def traverse[G[_], B](f: A => G[B])(implicit G: Applicative[G]): G[NonEmptyList[B]] =
G.map2Eval(f(head), Always(Traverse[List].traverse(tail)(f)))(NonEmptyList(_, _)).value

Expand Down Expand Up @@ -239,6 +245,9 @@ private[data] sealed trait NonEmptyListInstances extends NonEmptyListInstances0
override def reduceLeft[A](fa: NonEmptyList[A])(f: (A, A) => A): A =
fa.reduceLeft(f)

override def reduce[A](fa: NonEmptyList[A])(implicit A: Semigroup[A]): A =
fa.reduce

override def map[A, B](fa: NonEmptyList[A])(f: A => B): NonEmptyList[B] =
fa map f

Expand Down
5 changes: 5 additions & 0 deletions core/src/main/scala/cats/data/NonEmptyVector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,11 @@ private[data] sealed trait NonEmptyVectorInstances {
go(f(a))
NonEmptyVector.fromVectorUnsafe(buf.result())
}

override def toList[A](fa: NonEmptyVector[A]): List[A] = fa.toVector.toList

override def toNonEmptyList[A](fa: NonEmptyVector[A]): NonEmptyList[A] =
NonEmptyList(fa.head, fa.tail.toList)
}

implicit def catsDataEqForNonEmptyVector[A](implicit A: Eq[A]): Eq[NonEmptyVector[A]] =
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/scala/cats/instances/list.scala
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ trait ListInstances extends cats.kernel.instances.ListInstances {

override def foldM[G[_], A, B](fa: List[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
Foldable.iteratorFoldM(fa.toIterator, z)(f)

override def fold[A](fa: List[A])(implicit A: Monoid[A]): A = A.combineAll(fa)
Copy link
Contributor

@kailuowang kailuowang Jan 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably missed something but I am curious what gain we get from overriding this? I mean the
In Monoid.combineAll it's

as.foldLeft(empty)(combine)

In Foldable.fold it's

 foldLeft(fa, A.empty) { (acc, a) =>
    A.combine(acc, a)
 }

Copy link
Collaborator Author

@peterneyens peterneyens Jan 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused about that too.

When you run the MapMonoidBench, you can see that the first using Monoid.combineAll is considerably faster than the second.

That's because Monoid.combineAll is overridden in some instances, eg Map and more general Iterable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. that's what I suspected, but I only checked the List instance.


override def toList[A](fa: List[A]): List[A] = fa
}

implicit def catsStdShowForList[A:Show]: Show[List[A]] =
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/scala/cats/instances/map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ trait MapInstances extends cats.kernel.instances.MapInstances {

override def foldM[G[_], A, B](fa: Map[K, A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
Foldable.iteratorFoldM(fa.valuesIterator, z)(f)

override def fold[A](fa: Map[K, A])(implicit A: Monoid[A]): A =
A.combineAll(fa.values)

override def toList[A](fa: Map[K, A]): List[A] = fa.values.toList
}
// scalastyle:on method.length
}
4 changes: 4 additions & 0 deletions core/src/main/scala/cats/instances/set.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ trait SetInstances extends cats.kernel.instances.SetInstances {

override def foldM[G[_], A, B](fa: Set[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
Foldable.iteratorFoldM(fa.toIterator, z)(f)

override def fold[A](fa: Set[A])(implicit A: Monoid[A]): A = A.combineAll(fa)

override def toList[A](fa: Set[A]): List[A] = fa.toList
}

implicit def catsStdShowForSet[A:Show]: Show[Set[A]] = new Show[Set[A]] {
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/scala/cats/instances/stream.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ trait StreamInstances extends cats.kernel.instances.StreamInstances {

override def foldM[G[_], A, B](fa: Stream[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
Foldable.iteratorFoldM(fa.toIterator, z)(f)

override def fold[A](fa: Stream[A])(implicit A: Monoid[A]): A = A.combineAll(fa)

override def toList[A](fa: Stream[A]): List[A] = fa.toList
}

implicit def catsStdShowForStream[A: Show]: Show[Stream[A]] =
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/scala/cats/instances/vector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances {

override def foldM[G[_], A, B](fa: Vector[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
Foldable.iteratorFoldM(fa.toIterator, z)(f)

override def fold[A](fa: Vector[A])(implicit A: Monoid[A]): A = A.combineAll(fa)

override def toList[A](fa: Vector[A]): List[A] = fa.toList
}

implicit def catsStdShowForVector[A:Show]: Show[Vector[A]] =
Expand Down