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

Either.filterOrElse behavior is confusing #10044

Closed
scabug opened this issue Nov 11, 2016 · 2 comments
Closed

Either.filterOrElse behavior is confusing #10044

scabug opened this issue Nov 11, 2016 · 2 comments

Comments

@scabug
Copy link

scabug commented Nov 11, 2016

The behavior of Either.filterOrElse is confusing when the predicate doesn't match; it converts Either[A, B] to Either[AA, B] where AA is the "zero" element provided to filterOrElse. This is unexpected because for everything else, a filter operation results in a possible reduction of the number of elements in the container, and doesn't actually change the value of the elements. What I'd expect is to get an Option[Right] = None if the Either is a right and the predicate doesn't match and a Option[Right] = Some(a) otherwise. If it's a Left, the existing behavior of no-op is expected.
This causes subtle bugs in the code like the following:

def brokers(throw1: () => List[Int], throw2: List[Int] => List[String]) = {
  println("brokers ===> " +
    (Try(throw1())
      .toEither
      .filterOrElse(!_.isEmpty, Nil)
      .flatMap(xs => Try(throw2(xs)).toEither) match {
        case Right(s) => s
        case Left(f) => throw f.asInstanceOf[Throwable]
      })
    )
}

Call:
brokers(() => Nil, (xs: List[Int]) => throw new RuntimeException("throw2"))

Exception in thread "main" java.lang.ClassCastException: scala.collection.immutable.Nil$ cannot be cast to java.lang.Throwable at Practice$.brokers(Practice.scala:57) at Practice$.delayedEndpoint$Practice$1(Practice.scala:63)

First of all, the Throwable is lost causing a compile error (shown by the cast).
It converted the Either[Throwable,List[Int]] = Right(List()) to an Either[java.io.Serializable,List[Int]] = Left(List()). Thus the compile error, and the match with case Left and eventual exception. The Serializable must have come from the contravariant type parameter AA >: A, because the first common supertype for Throwable and List is a Serializable.

@scabug
Copy link
Author

scabug commented Nov 11, 2016

Imported From: https://issues.scala-lang.org/browse/SI-10044?orig=1
Reporter: Abhijit Sarkar (asarkar)
Affected Versions: 2.12.0

@scabug
Copy link
Author

scabug commented Nov 16, 2016

@SethTisue said:
I'd suggest starting a mailing list thread on this, and if you can gather some consensus on whether some change would be desirable, then come back to JIRA. filterOrElse is working as designed.

I'm closing the ticket, but I don't mean to dismiss your concern. See how the mailing list thread goes. (I already have some things in mind I will say on that thread if you start it.)

@scabug scabug closed this as completed Nov 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant