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.

Sign up for GitHub

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

Jump to bottom

Add Endo type alias for (A =>A) #2076

Merged
merged 4 commits into from
Dec 10, 2017
Merged

Add Endo type alias for (A =>A) #2076

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b51cc0f
added reduceRightK and Endo
kailuowang Dec 6, 2017
9410809
added mima excpetion
kailuowang Dec 6, 2017
7796307
removed reduceRightK
kailuowang Dec 7, 2017
828be44
minor
kailuowang Dec 7, 2017
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
4 changes: 3 additions & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,9 @@ def mimaSettings(moduleName: String) = Seq(
exclude[ReversedMissingMethodProblem]("cats.MonadError.rethrow"),
exclude[ReversedMissingMethodProblem]("cats.syntax.MonadErrorSyntax.catsSyntaxMonadErrorRethrow"),
exclude[DirectMissingMethodProblem]("cats.data.CokleisliArrow.id"),
exclude[IncompatibleResultTypeProblem]("cats.data.CokleisliArrow.id")
exclude[IncompatibleResultTypeProblem]("cats.data.CokleisliArrow.id"),
exclude[ReversedMissingMethodProblem]("cats.Foldable.reduceRightK"),
exclude[ReversedMissingMethodProblem]("cats.Foldable#Ops.reduceRightK")
)
}
)
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/scala/cats/Foldable.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,24 @@ import Foldable.sentinel
A.combine(acc, a)
}

/**
* Like `foldK` but from the right side.
* One use case would be endo functions (`A => A`) chaining
* because the default MonoidK instance for endo functions
* is using `compose` rather than the `andThen`
* {{{
* scala> import cats.implicits._, cats.Endo
* scala> val l: List[Endo[Int]] = List(_ * 2, _ + 3)
* scala> val f = l.reduceRightK //using `compose` to chain the functions
* scala> f(1)
* res0: Int = 5
* }}}
*/
def reduceRightK[G[_], A](fga: F[G[A]])(implicit G: MonoidK[G]): G[A] =
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like combineAllK to me.

Copy link
Contributor Author

@kailuowang kailuowang Dec 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

combineAllK is foldK right? This one combines in the opposite direction

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I misunderstood.

I am very -1 on this unless I am mistaken. reduceRight(Option) is right associative, but does not reverse the effective order that we iterate through. This method does by swapping.

I would call this reverseCombineAllK.

Since the MonoidK is associative, the result is the same if you right associate or left associate (though there may be efficiency reasons to pick one). This is something else: it is like making a new Monoid that has the property that combine(a, b) == original.combine(b, a)

This is actually still a monoid, but it behaves differently for non-commutative cases.

@kailuowang can you explain if I am wrong about this being different from reduceRight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right it's different from reduceRight(Option). reverseCombineAllK is probably a better name, but you also got to the bottom of it, we don't need it, we just need a different Monoid instance that does the original.combine(b, a). Maybe adding a reverse method to Monoid and MonoidK. But I am not sure are they worthwhile to be added?

What about the Endo type alias?

foldRight(fga, Eval.now(G.empty[A])) { (ga, ega) =>
ega.map(rga => G.combineK(rga, ga))
}.value

/**
* Alias for [[fold]].
*/
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/cats/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ package object cats {
* encodes pure unary function application.
*/
type Id[A] = A
type Endo[A] = A => A
implicit val catsInstancesForId: Bimonad[Id] with CommutativeMonad[Id] with Comonad[Id] with NonEmptyTraverse[Id] =
new Bimonad[Id] with CommutativeMonad[Id] with Comonad[Id] with NonEmptyTraverse[Id] {
def pure[A](a: A): A = a
Expand Down
1 change: 0 additions & 1 deletion tests/src/test/scala/cats/tests/CategorySuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import cats.laws.discipline.eq.catsLawsEqForFn1

class CategorySuite extends CatsSuite {
val functionCategory = Category[Function1]
type Endo[A] = Function1[A, A]

checkAll("Category[Function1].algebraK", MonoidKTests[Endo](functionCategory.algebraK).monoidK[Int])
checkAll("Category[Function1].algebraK", SerializableTests.serializable(functionCategory.algebraK))
Expand Down
1 change: 0 additions & 1 deletion tests/src/test/scala/cats/tests/ComposeSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import cats.laws.discipline.eq.catsLawsEqForFn1

class ComposeSuite extends CatsSuite {
val functionCompose = Compose[Function1]
type Endo[A] = Function1[A, A]

checkAll("Compose[Function1].algebraK", SemigroupKTests[Endo](functionCompose.algebraK).semigroupK[Int])
checkAll("Compose[Function1].algebraK", SerializableTests.serializable(functionCompose.algebraK))
Expand Down
11 changes: 10 additions & 1 deletion tests/src/test/scala/cats/tests/FoldableSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import cats.instances.all._
import cats.data._
import cats.laws.discipline.arbitrary._

abstract class FoldableSuite[F[_]: Foldable](name: String)(implicit ArbFInt: Arbitrary[F[Int]], ArbFString: Arbitrary[F[String]]) extends CatsSuite with PropertyChecks {
abstract class FoldableSuite[F[_]: Foldable](name: String)(
implicit ArbFInt: Arbitrary[F[Int]],
ArbFString: Arbitrary[F[String]],
ArbFListString: Arbitrary[F[List[String]]]) extends CatsSuite with PropertyChecks {

def iterator[T](fa: F[T]): Iterator[T]

Expand All @@ -33,6 +36,12 @@ abstract class FoldableSuite[F[_]: Foldable](name: String)(implicit ArbFInt: Arb
}
}

test("Foldable#reduceRightK") {
forAll { (fi: F[List[String]]) =>
fi.reduceRightK should === (fi.foldRight(Eval.now(List.empty[String]))((l, el) => el.map(_ ++ l)).value)
}
}

test("Foldable#partitionEither consistent with List#partition") {
forAll { (fi: F[Int], f: Int => Either[String, String]) =>
val list = Foldable[F].toList(fi)
Expand Down
3 changes: 1 addition & 2 deletions tests/src/test/scala/cats/tests/ReducibleSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cats
package tests

import org.scalacheck.Arbitrary

import cats.data.NonEmptyList

class ReducibleSuiteAdditional extends CatsSuite {
Expand Down Expand Up @@ -71,7 +70,7 @@ class ReducibleSuiteAdditional extends CatsSuite {

}

abstract class ReducibleSuite[F[_]: Reducible](name: String)(implicit ArbFInt: Arbitrary[F[Int]], ArbFString: Arbitrary[F[String]]) extends FoldableSuite[F](name) {
abstract class ReducibleSuite[F[_]: Reducible](name: String)(implicit ArbFInt: Arbitrary[F[Int]], ArbFString: Arbitrary[F[String]], ArbFListString: Arbitrary[F[List[String]]]) extends FoldableSuite[F](name) {
def range(start: Long, endInclusive: Long): F[Long]

test(s"Reducible[$name].reduceLeftM stack safety") {
Expand Down