ThoughtWorksInc / each

A macro library that converts native imperative syntax to scalaz's monadic expressions
Apache License 2.0
256 stars 25 forks source link

Strange behavior of `&&` operator (maybe also exists with other logic operation) #37

Open jilen opened 9 years ago

jilen commented 9 years ago

Code

monadic[Option] { val i = Some(1).each < 0; i && {println("foo"); Some(2).each > 0} }

Output

foo
res5: Option[Boolean] = Some(false)
Atry commented 9 years ago

Given a method that accepts a call-by-name parameter, it means the method decides when and how many times the parameter evaluates.

Boolean.&& is one of that kind of methods. Technically, there is no way to pass a monadic expression to methods like that. You could not pass an Option[Boolean] to a => Boolean typed method because type mismatch.

Instead, the only thing monadic macro can do is evaluating the monadic expression before passing it to a method. The following code is generated by monadic for you example code.

{
  val monad$macro$1: scalaz.Traverse[Option] with scalaz.MonadPlus[Option] with scalaz.Each[Option] with scalaz.Index[Option] with scalaz.Length[Option] with scalaz.Cozip[Option] with scalaz.Zip[Option] with scalaz.Unzip[Option] with scalaz.Align[Option] with scalaz.IsEmpty[Option] with scalaz.Cobind[Option] with scalaz.Optional[Option]{def point[A](a: => A): Some[A]; def cojoin[A](a: Option[A]): Option[Some[A]]} = scalaz.std.option.optionInstance;
  monad$macro$1.bind(com.thoughtworks.each.Monadic.EachOps[Some, Int](scala.Some.apply[Int](1)).underlying)(((element$macro$2: Int) => {
    val i: Boolean = element$macro$2.<(0);
    monad$macro$1.map({
      scala.this.Predef.println("foo");
      monad$macro$1.map(com.thoughtworks.each.Monadic.EachOps[Some, Int](scala.Some.apply[Int](2)).underlying)(((element$macro$3: Int) => element$macro$3.>(0)))
    })(((element$macro$4: Boolean) => i.&&(element$macro$4)))
  }))
}

In brief, there is no way to pass a monadic expression to a method that accept call-by-name parameters and still keep the same behavior as the behavior when passing a direct expression. No way, even if you manually create the monadic expression without using Each library.

Atry commented 9 years ago

In order to achieve the correspond behavior with a monadic expression, you must replace call-by-name parameter => T to a explicit function () => M[T], for example

// Monadic version of Boolean.&&
def monadicBooleanAnd[M[_] : Monad](leftHandSide: Boolean, rightHandSide: () => M[Boolean]): M[Boolean] = {
  if (leftHandSide) {
    rightHandSide()
  } else {
    Monad[M].point(false)
  }
}

monadic[Option] { val i = Some(1).each < 0; monadicBooleanAnd(i, { () => monadic[Option] { println("foo"); Some(2).each > 0 } }).each }
jilen commented 8 years ago

We should point this out in the README file. For a trade system like this will lead to terrible behavior

val decreased = account.decrease(xxx).each
val recordAdded = decreased && transfer.addRecord(xxx).each
Atry commented 8 years ago

Could you create a Pull Request to the README.md?

Atry commented 8 years ago

How do you think if we translate Boolean.&& to a if expressions to enable behavior like a call-by-name parameter?

jilen commented 8 years ago

That seems reasonable. And it will be better if we can raise an warning for other call-by-name parameter.

Atry commented 7 years ago

I think it's better to disable .each magic in a call-by-value parameters.

The following code should not compile any more

monadic[Option] { val i = Some(1).each < 0; i && {println("foo"); Some(2).each > 0} }
jilen commented 7 years ago

@Atry Can't agree anymore

MarisaKirisame commented 7 years ago

👍