Put method in trait or in case class?

Mikaël Mayer picture Mikaël Mayer · May 10, 2013 · Viewed 12.1k times · Source

There are two ways of defining a method for two different classes inheriting the same trait in Scala.

sealed trait Z { def minus: String }
case class A() extends Z { def minus = "a" }
case class B() extends Z { def minus = "b" }

The alternative is the following:

sealed trait Z { def minus: String = this match {
    case A() => "a"
    case B() => "b"
}
case class A() extends Z
case class B() extends Z

The first method repeats the method name, whereas the second method repeats the class name.

I think that the first method is the best to use because the codes are separated. However, I found myself often using the second one for complicated methods, so that adding additional arguments can be done very easily for example like this:

sealed trait Z {
  def minus(word: Boolean = false): String = this match {
    case A() => if(word) "ant" else "a"
    case B() => if(word) "boat" else "b"
}
case class A() extends Z
case class B() extends Z

What are other differences between those practices? Are there any bugs that are waiting for me if I choose the second approach?

EDIT: I was quoted the open/closed principle, but sometimes, I need to modify not only the output of the functions depending on new case classes, but also the input because of code refactoring. Is there a better pattern than the first one? If I want to add the previous mentioned functionality in the first example, this would yield the ugly code where the input is repeated:

sealed trait Z { def minus(word: Boolean): String  ; def minus = minus(false) }
case class A() extends Z { def minus(word: Boolean) = if(word) "ant" else "a" }
case class B() extends Z { def minus(word: Boolean) = if(word) "boat" else "b" }

Answer

Mik378 picture Mik378 · May 10, 2013

I would choose the first one.

Why ? Merely to keep Open/Closed Principle.

Indeed, if you want to add another subclass, let's say case class C, you'll have to modify supertrait/superclass to insert the new condition... ugly

Your scenario has a similar in Java with template/strategy pattern against conditional.

UPDATE:

In your last scenario, you can't avoid the "duplication" of input. Indeed, parameter type in Scala isn't inferable.

It still better to have cohesive methods than blending the whole inside one method presenting as many parameters as the method union expects.

Just Imagine ten conditions in your supertrait method. What if you change inadvertently the behavior of one of each? Each change would be risked and supertrait unit tests should always run each time you modify it ...

Moreover changing inadvertently an input parameter (not a BEHAVIOR) is not "dangerous" at all. Why? because compiler would tell you that a parameter/parameter type isn't relevant any more. And if you want to change it and do the same for every subclasses...ask to your IDE, it loves refactoring things like this in one click.

As this link explains:

Why open-closed principle matters:

No unit testing required.
No need to understand the sourcecode from an important and huge class.
Since the drawing code is moved to the concrete subclasses, it's a reduced risk to affect old functionallity when new functionality is added.

UPDATE 2:

Here a sample avoiding inputs duplication fitting your expectation:

sealed trait Z { 
     def minus(word: Boolean): String = if(word) whenWord else whenNotWord
     def whenWord: String
     def whenNotWord: String             
  }

case class A() extends Z { def whenWord = "ant"; def whenNotWord = "a"}

Thanks type inference :)