Happy Tuesday! Continue your great start to the week and check out this article by Software Engineer and Blockchain enthusiast, Łukasz Żuchowski 'Don't get mixed'.
Don’t get mixed
When I was a youngster at the beginning of my software development career I was taught by my master:
Prefer composition over inheritance, you should
Lego Yoda by Nigel Wade
So as the master said I did and I felt enlightened, my classes were simple and clean, their dependencies were straight and declared in constructor. Many years later I started programming in Scala — the new hope of the JVM. I found this language much more brief and powerful than Java at that time, the force was with me. Besides functional programming features there were couple of object-oriented ones as well such as traits.
Traits are interfaces on steroids, they allow not only to define the contract but also provide behavior and the killer feature was that you can inherit more that one. On the beginning it was a bit awkward but I felt guilty pleasure of using it. Mixing traits resolved many issues that I had before such as: adding markup interfaces, combining modules, inheriting two different contracts etc.
But as usual great power comes with great responsibility. In all the projects that I’ve seen traits were used in production code with caution. There were no bizarre mutants inheriting too many responsibility from multiple traits. There were no giants with hundred methods inherited from their ancestors. But the test code was a completely different story, somehow I and my mates forgot about the composition, and inheritance was the only way to go. Soon we had monsters like those:
trait AcceptanceSpec
extends FlatSpecLike
with TestFutureHelpers
with ScalaFutures
with BeforeAndAfterAll
with OptionValues
with TryValues
with EitherValues
with Eventually
with Matchers
with ConfigSupport
with LazyLogging
with AbstractPatienceConfiguration
abstract class StoreAcceptanceSpec
extends TestKit(ActorSystem("StoreAcceptanceSpec"))
with AcceptanceSpec
with StoreModule
with StoreConfigSupport
with PaymentSupport
with EcommerceDomainGenerators
with UserHelpers
with ProductSupport
with ProductHelpers
with OrdersFixtures
with StockSupport
'StoreAcceptanceSpec' had around 300 useful methods so it was very handy to use, like the Swiss Army knife.
The other issue is that traits have not only their abstract and concrete methods but they also have “dependencies”. In traits dependencies are usually expressed as an abstract non argument method e.g. 'def config:Config.'
Of course those traits weren’t like those from the beginning, they were growing incrementally inheriting more and more behavior. It’s also convenient to inherit ScalaTest traits such as 'ScalaFutures', 'BeforeAndAfterAll', 'BeforeAndAfterEach'. So maybe it’s not that bad?
Actually it is, because at some point you will by caught by the pitfalls:
1. Inheriting conflicting members — It is quite easy when you inherit plenty of traits to have conflicting members. Usually those are mentioned dependencies or implicits (e.g. patience config).
2. Issues with sequence of mixing — Some traits need to be mixed in the right order e.g. 'BeforeAndAfterEach' must be mixed on the end (at least not before any trait that extends 'Suite'), otherwise 'beforeEach' will never be executed. Of course it’s easy to maintain the order of mixins when there is only a couple of traits and inheritance hierarchy is one level tall. But take a look at this:
import org.scalatest.{BeforeAndAfterEach, FlatSpecLike, Suite}
trait BaseSpec extends Suite with BeforeAndAfterEach {
override protected def beforeEach(): Unit = {
super.beforeEach()
println("Before Each Base Spec")
}
}
abstract class ExtendedBaseSpec(val name:String) extends BaseSpec {
override protected def beforeEach(): Unit = {
super.beforeEach()
println(s"Before Extended Spec - $name")
}
}
class ExampleSpec extends ExtendedBaseSpec("ExampleSpec") with FlatSpecLike {
override protected def beforeEach(): Unit = {
super.beforeEach()
println("Before Each Example spec")
}
it should "test" in {
assert(condition = true)
}
}In this case 'beforeEach' will not be executed and the mixing order is forced by the fact that 'ExtendedBaseSpec' is an abstract class (if that was a trait, it would be the same but fixing the issue would be a bit more easier).
3. Class with 300 methods is something that you probably don’t need. At some point it might be very difficult to find method you want among 300 others and methods will depend one another. What you need is to import utility methods or build test dependencies.
4. Test that extends your module/application may also a bad practice. Overriding members and implicits could be troublesome for complex test hierarchies. What you really need is a service, your dependency. In most of the cases it should be easy to create instance of it in test. In more complex scenarios where you need to make instance of the module (provide its dependencies, override some members) code might be a bit more complex. If code like that will appear in many places you should consider creating object that represents it or factory to build it.
What are the alternatives?
The answer is obvious: composition. I would give you some advise like my master did, use it whenever it’s possible and natural. If you need a bunch of utility methods pack it in an object and import that object then you may use it like your own. If you need to build module to test it, build it and make it a dependency of your test, don’t inherit it. Your spec is not your module, so there is no point of extending it.
That’s the example how we may get rid off complex inheritance hierarchy. Of course I could import 'Matchers' object as well, but in this case I believe that making assertions is core responsibility of the test, so it’s worth to mix it in.
trait AcceptanceSpec
extends FlatSpecLike
with BeforeAndAfterEach
with Matchers
with LazyLogging
with AbstractPatienceConfiguration
abstract class StoreAcceptanceSpec
extends TestKit(ActorSystem("StoreAcceptanceSpec"))
with AcceptanceSpec
import EcommerceDomainGenerators.generateRandomOrder
import StoreSupport._
import ScalaFutures._
class BuyItemSpec extends AcceptanceSpec {
val mockUserDao = new MockUserDao()
val storeModule = new StoreModule() {
override def userDao = mockUserDao
}
override protected def beforeEach(): Unit = {
super.beforeEach()
mockUserDao.clear()
}
it should "buy something..." in {
// Given
val order = generateRandomOrder(UserHelpers.randomCustomer())
// When
val result = storeModule.orderService.placeOrder(order).futureValue
// Then
result should be(Right(OrderPlaced(order.id)))
}
}Disclaimer
I don’t want to point finger at anybody. I was author of those traits/spec as well and I did it couple of times till I learned. Actually in every Scala project I was a part of I met monsters I just described. On the other hand some libraries that we use encourage us to inherit many traits e.g. ScalaTest (but it’s not mandatory, you may import Objects). Using traits and mixing them is not a bad practice. Bad practice is using it where it is unjustified and could be replaced by composition.'
This article was written by Łukasz Żuchowski and posted originally on blog.softwaremill.com