You aren't going to need it

Writing code that is beyond specifications is not only a waste of time, but it also introduces unnecessary complexity. This complexity consists mainly in more code and tests to understand and maintain. Worse than that, superfluous code makes it harder to change code in the future. Let's consider an example given the following specifications:

  • Create a pedestrian traffic light allowing to walk if green, denying it if red
  • The traffic light can change color from red to green and vice versa
class TrafficLight(private var colour: TrafficLightColour) {

  fun changeColour() {
    colour = colour.changeColour()
  }

  fun possibleToWalk(): Boolean {
    return colour.possibleToWalk()
  }
}

enum class TrafficLightColour {
  RED {
    override fun changeColour() = GREEN
    override fun possibleToWalk() = false
  },
  GREEN {
    override fun changeColour() = RED
    override fun possibleToWalk() = true
  };

  abstract fun changeColour(): TrafficLightColour
  abstract fun possibleToWalk(): Boolean
}

The above code satisfies the specifications. However, one might say that traffic lights for pedestrians are usually timed: you press a button and it will take some seconds before the colour changes. This was not requested, but as it seems to be a more faithful representation of a pedestrian traffic light, so one might decide to go ahead like follows.

class TrafficLight(private var colour: TrafficLightColour) {

  fun changeColour(milliSecondsToWait: Long) {
    waitFor(milliSecondsToWait)
    colour = colour.changeColour(colour)
  }

  fun possibleToWalk(): Boolean {
    return colour.possibleToWalk()
  }

  private fun waitFor(milliSecondsToWait: Long) {
    Thread.sleep(milliSecondsToWait)
  }
}

enum class TrafficLightColour {
  RED {
    override fun changeColour() = GREEN
    override fun possibleToWalk() = false
  },
  GREEN {
    override fun changeColour() = RED
    override fun possibleToWalk() = true
  };

  abstract fun changeColour(): TrafficLightColour
  abstract fun possibleToWalk(): Boolean
}

Changing the behaviour of changeColour in TrafficLight had no benefits for what we are concerned with the current specifications. However, it added useless code inside the TrafficLight class and it forced all callers of the changeColour method to provide a value for milliSecondsToWait which, given the current specifications, will always be 0.


Recommended reads

Teach me back

I really appreciate any feedback about the book and my current understanding of software design.