16
\$\begingroup\$

I'm trying to come up with a scalable way of TDD'ing my Scala code, in particular managing dependencies. In this example, I've TDD'd part of the classic river crossing problem where we have people on each side of a river and move people from one bank to another.

I wanted to unit test the smaller functions on their own to avoid over-complicated tests. To be able to test the top level function, I've injected the functions it uses so that I can unit test them. Finally, I provide a method that does the dependency injection for external code.

Could a Scala pro comment on whether this is a reasonable thing to do?

package me.geoff.river_crossing

import me.geoff.library.ListExtensions._

object WorldTransform {

  def transform(world: World, move: Move): World = {
    transformF(world, move, getNewBoatPosition, updateBankWithMove)
  }

  type updateBankType = (List[Person], List[Person], Bank, Bank) => List[Person]

  def transformF(world: World, move: Move, getBank: Bank => Bank, updateBank: updateBankType): World = {
    val newBank = getBank(world.boatPosition)
    val leftBank = updateBank(world.leftBank, move.people, LeftBank(), world.boatPosition)
    val rightBank = updateBank(world.rightBank, move.people, RightBank(), world.boatPosition)
    World(leftBank, rightBank, newBank)
  }

  def getNewBoatPosition(bank: Bank): Bank = bank match {
    case LeftBank() => RightBank()
    case RightBank() => LeftBank()
  }

  def updateBankWithMove(bank: List[Person], people: List[Person], currentBank: Bank, thisBank: Bank): List[Person] = {
    if (currentBank == thisBank) bank.removeItems(people)
    else bank ++ people
  }
}

And the tests:

package me.geoff.river_crossing

import org.scalatest._
import org.scalatest.matchers.ShouldMatchers
import org.scalamock.scalatest.MockFactory

class TestWorldTransform extends FlatSpec with MockFactory with ShouldMatchers {

  trait Fixture {
    val mockGetBank = mockFunction[Bank, Bank]
    val mockUpdateBank = mockFunction[List[Person], List[Person], Bank, Bank, List[Person]]
    val leftBank = List(Cannibal())
    val rightBank = List(Missionary())
    val world = World(leftBank, rightBank, LeftBank())
    val move = Move(List(Cannibal()))
    def run() = {
      mockGetBank expects (LeftBank()) returning (RightBank())
      mockUpdateBank expects (leftBank, move.people, LeftBank(), LeftBank()) returning (List())
      mockUpdateBank expects (rightBank, move.people, RightBank(), LeftBank()) returning (List(Missionary(), Cannibal()))
      WorldTransform.transformF(world, move, mockGetBank, mockUpdateBank)
    }
  }

  "Transform" should "add result of getBank to result" in new Fixture {
    run().boatPosition should equal (RightBank())
  }

  it should "add result of left bank update to left bank" in new Fixture {
    run().leftBank should equal (List())
  }

  it should "add result of right bank update to right bank" in new Fixture {
    run().rightBank should equal (List(Missionary(), Cannibal()))
  }
}

class TestUpdateBankWithMove extends FlatSpec with ShouldMatchers {
  import WorldTransform.updateBankWithMove

  "Updating current bank" should "remove person from bank" in {
    val result = updateBankWithMove(List(Missionary()), List(Missionary()), LeftBank(), LeftBank())
    result should equal (Nil)
  }

  it should "leave person not in move" in {
    val result = updateBankWithMove(List(Missionary(), Cannibal()), List(Missionary()), LeftBank(), LeftBank())
    result should equal (List(Cannibal()))
  }

  "Updating opposite bank" should "add person to bank" in {
    val result = updateBankWithMove(List(), List(Missionary()), LeftBank(), RightBank())
    result should equal (List(Missionary()))
  }

  it should "include people already on bank" in {
    val result = updateBankWithMove(List(Cannibal()), List(Missionary()), LeftBank(), RightBank())
    result should equal (List(Cannibal(), Missionary()))
  }
}

class TestGetNewBoatPosition extends FlatSpec with ShouldMatchers {
  import WorldTransform.getNewBoatPosition

  "Get new boat position " should "return right bank for left bank" in {
    getNewBoatPosition(LeftBank()) should equal (RightBank())
  }

  it should "return left bank for right bank" in {
    getNewBoatPosition(RightBank()) should equal (LeftBank())
  }
}
\$\endgroup\$
2
\$\begingroup\$

I'm no Scala Pro, but I'll try to give my two cents anyway:

Your decision to "inject the functions" (pass them as parameters) in order to be able to test them resulted in awkward code, and complicated an otherwise simple method.

As for the tests you chose to make of the resulting method, they seem to only check that the functions are called - these tests are very brittle, since they assume implementation, and don't add much to actually test the code, they simply echo it...

Better tests would be to make a move on a starting state, and then check if the resulting state is as expected. This would also make redundant the need to use function injection in production code...

Also, if you really need to stub out functions within your code, I believe there are better ways to do it using generated mocks or spies.

| improve this answer | |
\$\endgroup\$

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.