Why does scalaz's implementation of Monoid for Option evaluate the f2 function twice?

Question

The definition of the scalaz's option monoid is as follows:

implicit def optionMonoid[A: Semigroup]: Monoid[Option[A]] = new Monoid[Option[A]] {
  def append(f1: Option[A], f2: => Option[A]) = (f1, f2) match {
    case (Some(a1), Some(a2)) => Some(Semigroup[A].append(a1, a2))
    case (Some(a1), None)     => f1
    case (None, Some(a2))     => f2
    case (None, None)         => None
  }

  def zero: Option[A] = None
}

f2 is a pass by name param which means each call will evaluate the expression. Why should it be evaluated again when it was just evaluated in the pattern match? Returning Some(a2) should be the same result and the expression f2 could be very expensive.

Am I missing something?

Scalaz's Option.scala source

Sialoid

Probably a carryover from Haskell thinking where the equivalent definition is cheap? – Intercalate

Did you test this? Put some println inside it and check. – Ulrich

Yes I did test it and the println is hit twice. I'll fix it and submit a pull request to scalaz I guess. – Sialoid

Answer 1

It looks to me like it was written to highlight the symmetry of the problem and for clarity, not for speed. You can't just drop the laziness of the second argument since Semigroup defines it that way, and in other contexts times the laziness of the second argument may be essential. To preserve the visual representation of the symmetry of the problem, you probably want to just add

val g2 = f2  // Force evaluation
(f1, g2) match { ...

or somesuch.

(It would be nice if by name arguments could be called lazy to automatically memoize them.)

Pled

I submitted it as a pull request and they merged it, but your solution would work as well. github.com/scalaz/scalaz/pull/352 – Sialoid