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
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 30, 2017 at 7:47
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Could you please add the definition of Calculation, which is returned from eval ? \$\endgroup\$ Commented Mar 30, 2017 at 9:27

1 Answer 1

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.

answered Apr 8, 2017 at 18:37
\$\endgroup\$
1
  • \$\begingroup\$ It may be worth having Failure[A] instead be case class Failure(errors: List[String]) extends Calculation[Nothing] \$\endgroup\$ Commented May 12, 2017 at 3:50

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.