3
\$\begingroup\$

I am learning Scala and want to know, what is preferred style of writing a function for a trait.

First:

sealed trait Expression

final case class Addition(left: Expression, right: Expression) extends Expression

final case class Subtraction(left: Expression, right: Expression) extends Expression

final case class Division(left: Expression, right: Expression) extends Expression

final case class SquareRoot(value: Expression) extends Expression

final case class Number(value: Double) extends Expression

object Expression {

  def eval(e: Expression): Calculation =
    e match {
      case Addition(l1, r1) => eval(l1) match {
        case Failure(rea1) => Failure(rea1)
        case Success(res1) => eval(r1) match {
          case Failure(rea2) => Failure(rea2)
          case Success(res2) => Success(res1 + res2)
        }
      }
      case Subtraction(l1, r1) => eval(l1) match {
        case Failure(rea1) => Failure(rea1)
        case Success(res1) => eval(r1) match {
          case Failure(rea2) => Failure(rea2)
          case Success(res2) => Success(res1 - res2)
        }
      }
      case Division(l1, r1) => eval(r1) match {
        case Failure(rea2) => Failure(rea2)
        case Success(res2) => res2 match {
          case 0.0 => Failure("Division by zero")
          case _ => eval(l1) match {
            case Failure(rea1) => Failure(rea1)
            case Success(res1) => Success(res1 / res2)
          }
        }
      }
      case SquareRoot(v) => eval(v) match {
        case Failure(rea) => Failure(rea)
        case Success(res) => res match {
          case x if x < 0 => Failure("Square root of negative number")
          case y => Success(math.sqrt(y))
        }
      }
      case Number(v) => Success(v)
    }

}

Second:

sealed trait Expression {
  def eval: Calculation =
    this match {
      case Addition(l, r) =>
        l.eval match {
          case Failure(reason) => Failure(reason)
          case Success(r1) =>
            r.eval match {
              case Failure(reason) => Failure(reason)
              case Success(r2) => Success(r1 + r2)
            }
        }
      case Subtraction(l, r) =>
        l.eval match {
          case Failure(reason) => Failure(reason)
          case Success(r1) =>
            r.eval match {
              case Failure(reason) => Failure(reason)
              case Success(r2) => Success(r1 - r2)
            }
        }
      case Division(l, r) =>
        l.eval match {
          case Failure(reason) => Failure(reason)
          case Success(r1) =>
            r.eval match {
              case Failure(reason) => Failure(reason)
              case Success(r2) =>
                if(r2 == 0)
                  Failure("Division by zero")
                else
                  Success(r1 / r2)
            }
        }
      case SquareRoot(v) =>
        v.eval match {
          case Success(r) => if(r < 0)
            Failure("Square root of negative number")
          else
            Success(Math.sqrt(r))
          case Failure(reason) => Failure(reason)
        }
      case Number(v) => Success(v)
    }
}
final case class Addition(left: Expression, right: Expression) extends Expression
final case class Subtraction(left: Expression, right: Expression) extends Expression
final case class Division(left: Expression, right: Expression) extends Expression
final case class SquareRoot(value: Expression) extends Expression
final case class Number(value: Int) extends Expression
\$\endgroup\$
  • 2
    \$\begingroup\$ Could you please add the definition of Calculation, which is returned from eval ? \$\endgroup\$ – Antot Mar 30 '17 at 9:27
1
\$\begingroup\$

In terms of you where to put the eval function, both are fine really. I have seen bits of both in libraries likes cats or scalaz. And given how easily you get one from the other, it does not make a massive difference architecturally.

However, I see a few issues on the side of validating your expressions, that I am honestly acking to improve :)

1) Both sides of an expression can result in an error, but you will show only one of them to the user.

2) These nested match-case expressions can be massively simplified by using one of the many functional validation libraries.

Let me attempt to solve both of these by using Scalactic's validation:

import org.scalactic._
import Accumulation._

object Arithmetics extends App {

  sealed trait Expression {
    def eval: Double Or Every[String] = this match {
      case Addition(l, r) => withGood(l.eval, r.eval)(_ + _)
      case Subtraction(l, r) => withGood(l.eval, r.eval)(_ - _)
      case Division(l, r) =>
        def isNonZero(i: Double) = if (i != 0) Pass else Fail(s"divisor evaluated to zero: $r")
        withGood(l.eval, r.eval.when(isNonZero))(_ / _)
      case SquareRoot(v) =>
        def isGreaterThanZero(i: Double) = if (i >= 0) Pass else Fail(s"Expression under square root evaluated to $i (<0): $v")
        v.eval.when(isGreaterThanZero).map(Math.sqrt)
      case Number(v) => Good(v)
    }
  }
  final case class Addition(left: Expression, right: Expression) extends Expression
  final case class Subtraction(left: Expression, right: Expression) extends Expression
  final case class Division(left: Expression, right: Expression) extends Expression
  final case class SquareRoot(value: Expression) extends Expression
  final case class Number(value: Int) extends Expression

  val failing = Addition(SquareRoot(Number(-5)), Division(Number(1), Subtraction(Number(1), Number(1))))
  println(failing.eval)
  // Bad(Many(Expression under square root evaluated to -5.0 (<0): Number(-5), divisor evaluated to zero: Subtraction(Number(1),Number(1))))

  val valid = Addition(SquareRoot(Number(25)), Division(Number(1), Number(2)))
  println(valid.eval)
  //Good(5.5)
}

You can see that you can get multiple errors at once, and much fewer lines of code, plus a human readable syntax.

In case you want to stick with your custom Calculation, it could look like this:

object Arithmetics2 extends App {

  sealed trait Calculation[A] {
    def map[B](f: A => B): Calculation[B] = this match {
      case Success(value) => Success(f(value))
      case Failure(errors) => Failure(errors)
    }
    def zip[B](other: Calculation[B]): Calculation[(A, B)] = (this, other) match {
      case (Success(valueA), Success(valueB)) => Success((valueA,valueB))
      case (Failure(errorsA), Failure(errorsB)) => Failure(errorsA ++ errorsB)
      case (_, Failure(errors)) => Failure(errors)
      case (Failure(errors), _) => Failure(errors)
    }
    def filter(f: A => Boolean, msg: A => String): Calculation[A] = this match {
      case Success(value) if f(value) => Success(value)
      case Success(value) => Failure(List(msg(value)))
      case Failure(errors) => Failure(errors)
    }
  }
  case class Success[A](value: A) extends Calculation[A]
  case class Failure[A](errors: List[String]) extends Calculation[A]


  sealed trait Expression {
    def eval: Calculation[Double] = this match {
      case Addition(l, r) => l.eval.zip(r.eval).map{ case (a,b) => a + b}
      case Subtraction(l, r) => l.eval.zip(r.eval).map{ case (a,b) => a - b}
      case Division(l, r) =>
        l.eval.zip(r.eval.filter(_ != 0, _ => s"divisor evaluated to zero: $r")).map{ case (a,b) => a / b}
      case SquareRoot(v) =>
        v.eval.filter(_ >= 0, i => s"Expression under square root evaluated to $i (<0): $v").map(Math.sqrt)
      case Number(v) => Success(v)
    }
  }
  final case class Addition(left: Expression, right: Expression) extends Expression
  final case class Subtraction(left: Expression, right: Expression) extends Expression
  final case class Division(left: Expression, right: Expression) extends Expression
  final case class SquareRoot(value: Expression) extends Expression
  final case class Number(value: Int) extends Expression

  val failing = Addition(SquareRoot(Number(-5)), Division(Number(1), Subtraction(Number(1), Number(1))))
  println(failing.eval)
  // Failure(List(Expression under square root evaluated to -5.0 (<0): Number(-5), divisor evaluated to zero: Subtraction(Number(1),Number(1))))

  val valid = Addition(SquareRoot(Number(25)), Division(Number(1), Number(2)))
  println(valid.eval)
  //Success(5.5)
}

Notice that it actually is not that lengthy/hard to make some decent validation mechanism. Once you get a bit more into functional programming, you will see that functions like map, filter or zip can be found in many places. You will find them for example on most collections, like List.

\$\endgroup\$
  • \$\begingroup\$ It may be worth having Failure[A] instead be case class Failure(errors: List[String]) extends Calculation[Nothing] \$\endgroup\$ – Morgen May 12 '17 at 3:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.