Connecting...

W1siziisimnvbxbpbgvkx3rozw1lx2fzc2v0cy9zawduawz5lxrly2hub2xvz3kvanbnl2jhbm5lci1kzwzhdwx0lmpwzyjdxq

'Don’t get mixed' by Łukasz Żuchowski

W1siziisijiwmtgvmtevmtmvmtavmtkvmzivnjexl3blegvscy1wag90by01nzqwnjkuanblzyjdlfsiccisinrodw1iiiwiotawedkwmfx1mdazzsjdxq

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