Keep your class OCP compliant by using enums instead of booleans

Open for extension closed for modification. This means that our code is written in such a way that our class can have new behaviour without changing its structure.

Lets say that we have a Task and this task can have a description, a status and a couple of on/off characteristics like the fact that can be edited, shared or tracked.

Using flags

One way to depict this in code is by using flags in the form of booleans:

class Task(
val description: String,
val status: Status,
val isEditable: Boolean,
val isTrackable: Boolean,
val isSharable: Boolean
)

This means that every time we need to add a new characteristic we have to modify the Task class and add a new boolean property. This violates the open close principle making the code difficult to scale.

Using a list of enums

Another way is to replace all these flags with a list of enums:

class Task(
val description: String,
val status: Status,
private val characteristics: List<Characteristic>
) {
fun hasCharacteristic(characteristic: Characteristic): Boolean {
return characteristic in characteristics
}
}
enum class Characteristic {
Editable,
Trackable,
Sharable
}

Now if we need to add a new characteristic we simply add a new enum entry and the entire project can keep using the hasCharacteristic method to query a task for its characteristics.

Bonus benefit: this way we also avoid the wall of properties that sometimes hides information instead of revealing it!

LSP and ISP: The LI of SOLID

I am under the impression that every time I come across a SOLID post, LSP and ISP are not given the same amount of attention as the other three. I guess its because they are the easiest to grasp but make no mistake, they are also the easiest to violate!

Liskov Substitution Principle (LSP)

In essence, LSP proposes that we create sub-classes that can be used wherever their parents are being used without breaking or changing the client’s behavior.

This means that in the code snippet below:

fun printSeniority(engineer: SoftwareEngineer, yearsInBusiness: Int) {
val seniority = engineer.calculateSeniority(yearsInBusiness)
val label = when (seniority) {
SoftwareEngineer.JUNIOR -> "junior"
SoftwareEngineer.MID -> "mid"
else -> "senior"
}
println("This is a $label engineer")
}

we should be able to pass instances from all sub-classes of SoftwareEngineer without worrying that calculateSeniority will break or change the behavior of printSeniority.

Ways that we tend to violate LSP
  • By throwing an exception
class MobileEngineer : SoftwareEngineer() {
override fun calculateSeniority(yearsInBusiness: Int): Int {
require(yearsInBusiness > 2)
return super.calculateSeniority(yearsInBusiness)
}
}

This is the obvious one. If, for example, the subclass adds a check that will eventually throw an exception then the client will break.

  • By returning undocumented results
class MobileEngineer : SoftwareEngineer() {
override fun calculateSeniority(yearsInBusiness: Int): Int {
if (yearsInBusiness >= 15) {
return ULTRA_SENIOR
}
return super.calculateSeniority(yearsInBusiness)
}
companion object {
const val ULTRA_SENIOR = 3
}
}
// which leads in:
fun printSeniority(engineer: SoftwareEngineer, yearsInBusiness: Int) {
val seniority = engineer.calculateSeniority(yearsInBusiness)
val label = when (seniority) {
MobileEngineer.ULTRA_SENIOR -> "ultra senior" // printSeniority is forced to know about MobileEngineer
SoftwareEngineer.JUNIOR -> "junior"
SoftwareEngineer.MID -> "mid"
else -> "senior"
}
println("This is a $label engineer")
}

In other words, the subclass returns something that the its parent never will.
This forces the client to know about the subclass which makes the code less scalable.

  • By having side effects
class MobileEngineer(
private val backend: Backend
) : SoftwareEngineer() {
override fun calculateSeniority(yearsInBusiness: Int): Int {
return backend.calculateSeniority(yearsInBusiness)
}
}

This is the subtle one since it does not change the client’s code but it does change the expected behavior. printSeniority is expected to make a calculation and then print the result but know it also makes a network call!

Interface Segregation Principle (ISP)

In essence, ISP proposes that interfaces should not play the role of “methods bucket” because eventually there will be a client that will not need to implement all of them.

This means that interfaces like this:

interface Repository {
// db:
fun select(id: Int): SoftwareEngineer?
fun insert(engineer: SoftwareEngineer)
// api:
fun get(id: Int): SoftwareEngineer?
fun post(engineer: SoftwareEngineer)
}
class LocalEngineersCache : Repository {
override fun select(id: Int): SoftwareEngineer? {
// actual implementation
}
override fun insert(engineer: SoftwareEngineer) {
// actual implementation
}
override fun get(id: Int): SoftwareEngineer? {
throw UnsupportedOperationException()
}
override fun post(engineer: SoftwareEngineer) {
throw UnsupportedOperationException()
}
}

should break in more meaningful parts and allow every client to implement only the part that it needs:

interface Database {
fun select(id: Int): SoftwareEngineer?
fun insert(engineer: SoftwareEngineer)
}
interface Api {
fun get(id: Int): SoftwareEngineer?
fun post(engineer: SoftwareEngineer)
}
class LocalEngineersCache : Database {
override fun select(id: Int): SoftwareEngineer? {
// actual implementation
}
override fun insert(engineer: SoftwareEngineer) {
// actual implementation
}
}

Its worth mentioning that this way, in addition from avoiding ISP violation we also:

  1. Keep our code from violating the SRP.
    In the first implementation, our cache depends in two things so it has more that one reasons to change (ex: a new parameter in the post method would force us to change our cache too)
  2. Keep our code from violating the LSP. By having one interface, the first implementation of our cache couldn’t be used in code that expects repositories since its API methods would break the client.
  3. Keep our code clean and scalable (the cache does not have to know about talking to the API)
Ways that we tend to violate ISP

Although there is not much to say here, since the only way to do it is by creating those buckets we mentioned above, beware of the creation since it comes in two flavors.
The first is by having all methods in one file. Easy to catch it in a PR review. The second though is by having an hierarchy in our interfaces.

interface Load {
fun load(): List<SoftwareEngineer>
}
interface LoadAndSave : Load {
fun save(engineer: SoftwareEngineer)
}

By the time there is an implementation that does not need all methods, it might too late!