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

Try.map violates functor composition law #6284

Closed
scabug opened this issue Aug 26, 2012 · 9 comments
Closed

Try.map violates functor composition law #6284

scabug opened this issue Aug 26, 2012 · 9 comments
Labels

Comments

@scabug
Copy link

@scabug scabug commented Aug 26, 2012

For all functions f and g,
Success(a) map { f compose g }
should produce the same result as
Success(a) map g map f

However, in the following example:

def divideByZero(a: Int): Int = a / 0
def numberOrDefault(a: => Int): Int =
  try a catch { case _: ArithmeticException => 42 }

the property we want is not satisfied:

Success(1) map { numberOrDefault compose divideByZero } === Success(42)
Success(1) map divideByZero map NumberOrDefault === Failure(ArithmeticException)

In order to satisfy the functor composition law in the above case, we need something like the following implementation for Failure.map:
def map[U](f: T => U): Try[U] = Try[U](f(throw exception))

This allows the possibility of going from a Failure to a Success, which is correct if Try is attempting to mimic the semantics of exception handling.

EDIT: The same can be said for Failure.flatMap, by the way. It should also be possible for the function passed to flatMap to catch the contained exception and produce a Success, as above.

@scabug
Copy link
Author

@scabug scabug commented Aug 26, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6284?orig=1
Reporter: Dan Rosen (mergeconflict)
Affected Versions: 2.10.0-M6

@scabug
Copy link
Author

@scabug scabug commented Aug 26, 2012

@jsuereth said:
This is only true for the category of types and functions. If you limit your category to that of types and functions (arrows) that do not catch nonfatal exceptions, then there is no issue. Since try is reifying exception handling, this is not an unreasonable assertion to make.

@scabug
Copy link
Author

@scabug scabug commented Aug 26, 2012

Dan Rosen (mergeconflict) said:
{quote}Since try is reifying exception handling, this is not an unreasonable assertion to make.{quote}

In the case of flatMap, I'd probably buy that argument. That is, when you have f: A => Try[B] it's probably reasonable to assume that whoever wrote f knows how Try is supposed to work! But there are two problems:

  • If that were the case, then the implementation of Success.flatMap should simply be f(value), right? No need to try/catch, since f presumably won't throw a non-fatal exception.
  • More importantly, I think in the case of map that your assertion doesn't fly. That is, you can't assume the author of some arbitrary, pre-existing f: A => B will have foreknowledge that their function will be lifted into the Try functor, nor is it safe to assume that the consumer of f can see its implementation. If you're writing all the code yourself, no problem, but you know how that goes...
@scabug scabug closed this Aug 26, 2012
@scabug
Copy link
Author

@scabug scabug commented Aug 26, 2012

@odersky said:
I think this comment and the discussion should be directed to the SIP 14 mailing list (scala-sips@googlegroups.com). It's a design decision, not a bug.

@scabug
Copy link
Author

@scabug scabug commented Aug 26, 2012

Dan Rosen (mergeconflict) said:
Sorry, I figured the design was already pretty well solidified following the discussion in the pull request: scala/scala#1056 ... I assumed it was a bug, but if not I won't follow up on the list. Thanks!

@scabug
Copy link
Author

@scabug scabug commented Aug 27, 2012

Peter Pnin (pnin) said:
The example above doesn't compile on scala 2.10-M6.

Success(1) map { numberOrDefault compose divideByZero }
:13: error: missing arguments for method numberOrDefault;
follow this method with `_' if you want to treat it as a partially applied function
Success(1) map { numberOrDefault compose divideByZero }

@scabug
Copy link
Author

@scabug scabug commented Oct 11, 2014

Andrii Polishchuk (rgtbctltpx) said (edited on Oct 11, 2014 10:57:05 PM UTC):
The code is broken, and if you fix it, you will see that Try is actually a valid functor.
Here are two possible ways to fix it:

  1. http://ideone.com/Z21SUn
import scala.util._

object Main extends App {
  def divideByZero(a: Int): Int = a / 0
  def numberOrDefault(a: Int): Int =
    try a catch { case _: ArithmeticException => 42 }
    
  println(Success(1) map { numberOrDefault _ compose divideByZero _ })
  println(Success(1) map divideByZero _ map numberOrDefault _)
}
  1. http://ideone.com/Z9zePL
import scala.util._

object Main extends App {
  def divideByZero(a: Int): Int = a / 0
  def numberOrDefault(a: => Int): Int =
    try a catch { case _: ArithmeticException => 42 }
    
  println(Success(1) map { ((x: Int) => numberOrDefault(x)) compose divideByZero _ })
  println(Success(1) map divideByZero _ map ((x: Int) => numberOrDefault(x)))
}

You may argue this is the right example: http://ideone.com/TgkLCO

import scala.util._

object Main extends App {
  def divideByZero(a: Int): Int = a / 0
  def numberOrDefault(a: => Int): Int =
    try a catch { case _: ArithmeticException => 42 }
    
  println(Success(1) map { x => numberOrDefault(divideByZero(x)) })
  println(Success(1) map divideByZero _ map ((x: Int) => numberOrDefault(x)))
}

But that's not true, because for "map (f . g)" case you use

f = numberOrDefault
g = divideByZero

and for "map g . map f" case you use another f:

f = ((x: Int) => numberOrDefault(x))
g = divideByZero

By this example you actually prove that numberOrDefault !== ((x: Int) => numberOrDefault( x )), which is quite obvious fact.

@scabug
Copy link
Author

@scabug scabug commented Oct 28, 2014

@jsuereth said:
Just since I saw some activity here, Wanted to point out that the tests are super invalid because they throw exceptions. For the Functor laws, you need to define your category first. I assume you mean a category EQUIVALENT with "HASK" which also fails most of the Monadic/Functor laws.

If you ignore Bottom type, or otherwise throwing an exception as a possibility in functions, then the laws hold. See: http://www.haskell.org/haskellwiki/Hask "Platonic Hask".

Again, claiming category theory laws really needs a category definition first, otherwise all of this postulation is meaningless.

@scabug
Copy link
Author

@scabug scabug commented Oct 21, 2016

Jason Hu (HuStmpHrrr) said (edited on Oct 21, 2016 7:23:02 PM UTC):
there are a number of mistakes you are making here i think.

first of all, the so-called composition you are making is bogus. numberOrDefault is actually taking a closure, and composing divideByZero actually means to capture it inside of a closure, instead of the expression itself. composition law of functor doesn't specify this situation at all. that is

tryy map { p => g(() => f(p)) }

doesn't have any assumption of relation to

tryy map f map g

but rather should be equivalent to

tryy map { p => () => f(p) } map g

don't let syntax trick you.

also, another problem is Try is not a valid point of discussion anyways. Try handles side effects and expects ones. it's ok to not have this class escape from the laws.

@scabug scabug added the won't fix label Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.