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
	)

view raw task-with-flags.kt hosted with ❤ by GitHub

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
	}

view raw task-with-enums.kt hosted with ❤ by GitHub

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!

The “Tell, don’t ask” principle

This is one of my favorite principles because it is easy to spot when violated and because it helps in having proper API surfaces when applied.

Tell, don’t ask

In essence this principle proposes that instead of asking from an instance for its values in order to decide how the same instance will execute something, just tell the instance to execute it. It knows its own state, it can make its own decisions!

Asking

By asking we actually refer to accessing many of a class’s properties. For example:

	// somewhere in the code
	if (task.status == Status.NotStarted && task.createdAt < LocalDate.of(2021, 1, 14) && task.subscribers.isEmpty()) {
	task.close()
	}

	// which might have led to this API
	class Task(
	initialStatus: Status,
	val createdAt: LocalDate
	) {

	var status: Status = initialStatus
	private set

	private val innerSubscribers = mutableListOf<Subscriber>()
	val subscribers: List<Subscriber> = innerSubscribers

	fun add(subscriber: Subscriber) {
	innerSubscribers.add(subscriber)
	}

	fun close() {
	status = Status.Resolved
	}
	}

view raw tell_dont_ask__violation.kt hosted with ❤ by GitHub

in the code above we ask the task to provide the values of three of its properties in order to decide if we are going to close it or not.
In the process we (a) might had to expose those properties just for this code (creation date and subscribers could be private) and (b) inevitably leaked business logic that involves a task (when a task is eligible for closing).

Telling

Lets change the code in order to tell the task to close itself if possible:

	// somewhere in the code
	task.closeIfUnclaimed(createdBefore = LocalDate.of(2021, 1, 14))

	// task's better API
	class Task(
	initialStatus: Status,
	private val createdAt: LocalDate
	) {

	var status: Status = initialStatus
	private set

	private val subscribers = mutableListOf<Subscriber>()

	fun add(subscriber: Subscriber) {
	subscribers.add(subscriber)
	}

	fun closeIfUnclaimed(createdBefore: LocalDate): Boolean {
	if (status == Status.NotStarted && createdAt < createdBefore && subscribers.isEmpty()) {
	status = Resolved
	return true
	}

	return false
	}
	}

view raw tell_dont_ask__applied.kt hosted with ❤ by GitHub

There are two things that we gain from this change:

we moved the “closing” logic in the Task itself which allows us to contain future logic alterations in just one place.
the class’s API exposes only the necessary helping us to avoid accidental couplings.

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")
	}

view raw lsp_isp__lsp_fun.kt hosted with ❤ by GitHub

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)
	}
	}

view raw lsp_isp__lsp_throw.kt hosted with ❤ by GitHub

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")
	}

view raw lsp_isp__lsp_know_more.kt hosted with ❤ by GitHub

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)
	}
	}

view raw lsp_isp__lsp_network.kt hosted with ❤ by GitHub

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()
	}
	}

view raw lsp_isp__isp_all_in_one.kt hosted with ❤ by GitHub

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
	}
	}

view raw lsp_isp__isp_broken.kt hosted with ❤ by GitHub

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

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)
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.
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)
	}

view raw lsp_isp__isp_hierarchy.kt hosted with ❤ by GitHub

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

IoC: Inversion of Control principle

This is the one principle that, chances are, you have applied even if you didn’t do it on purpose.
In essence, if you have written code that does not have any control over the execution flow, then you most probably have applied the IoC principle. How?

IoC through design patterns

If you have implemented the strategy or the template pattern then you have applied the IoC principle. For example, having a report generator and feeding it with the necessary filtering code.
All the code you write for filtering data, does not have any control over the execution’s flow. The generator is the one that decides when and if it will be invoked:

	class Report(
	val women: List<Person>,
	val men: List<Person>
	) {
	fun isEmpty(): Boolean {
	return women.isEmpty() && men.isEmpty()
	}
	}

	class ReportGenerator(
	private val repository: Repository
	) {

	fun create(filter: (Report) -> Report): Report {
	val people = repository.fetchAllPeople()

	val rawReport = splitByGender(people)
	if (rawReport.isEmpty()) {
	return rawReport
	}

	// the filter function does not have any control over its invokation
	val filteredReport = filter(rawReport)
	return sortByName(filteredReport)
	}

	// …
	}

	// example:
	val reportGenerator = ReportGenerator(repository)

	val withPeopleOver21 = { rawReport: Report ->
	val over21 = { person: Person -> person.age > 21 }
	Report(
	rawReport.women.filter(over21),
	rawReport.men.filter(over21)
	)
	}

	val reportWithPeopleOver21 = reportGenerator.create(withPeopleOver21)

view raw ioc__strategy_pattern.kt hosted with ❤ by GitHub

Strategy pattern

The same goes with the template pattern too. The code you write in the template’s hooks is being controlled by the template and you don’t get to control it:

	class Report(
	val women: List<Person>,
	val men: List<Person>
	) {
	fun isEmpty(): Boolean {
	return women.isEmpty() && men.isEmpty()
	}
	}

	abstract class ReportGenerator(
	private val repository: Repository
	) {

	fun create(): Report {
	val people = repository.fetchAllPeople()

	val rawReport = splitByGender(people)
	if (rawReport.isEmpty()) {
	return rawReport
	}

	val filteredReport = filter(rawReport)
	return sortByName(filteredReport)
	}

	abstract fun filter(report: Report): Report

	// …
	}

	class AdultsReportGenerator(repository: Repository) : ReportGenerator(repository) {

	override fun filter(report: Report): Report {
	val over21 = { person: Person -> person.age > 21 }
	return Report(
	report.women.filter(over21),
	report.men.filter(over21)
	)
	}
	}

	// example:
	val generator = AdultsReportGenerator(repository)
	val reportWithPeopleOver21 = generator.create()

view raw ioc__template_pattern.kt hosted with ❤ by GitHub

Template pattern

Both of these examples might seem weird since we are the ones that wrote both the filtering code and the generator so we feel that we control the flow. The thing is that we need to separate them in our heads and observe them individually. The generator is the one that controls the flow and dictates the actions that will take place. The filtering code is one of the actions. We just write them, provide them to the generator and that’s it.

IoC through frameworks

Another way of applying IoC is by using frameworks that have adopted it. Most popular are the IoC containers that are used to inject dependencies.
Another example is the android’s framework. In android you don’t have control over an activity’s lifecycle and you simply extend the Activity class and override hooks to run your code.

IoC vs DI

Because of the aforementioned IoC containers, many people assume that dependency injection and IoC are the same. They are not. DI is just a way to help in applying the IoC principle. For example:

	class Report(
	val women: List<Person>,
	val men: List<Person>
	) {
	fun isEmpty(): Boolean {
	return women.isEmpty() && men.isEmpty()
	}
	}

	class ReportGenerator(
	private val repository: Repository,
	private val filter: (Report) -> Report
	) {

	fun create(): Report {
	val people = repository.fetchAllPeople()

	val rawReport = splitByGender(people)
	if (rawReport.isEmpty()) {
	return rawReport
	}

	val filteredReport = filter(rawReport)
	return sortByName(filteredReport)
	}

	// …
	}

	// Example:
	val withPeopleOver21 = { report: Report ->
	val over21 = { person: Person -> person.age > 21 }
	Report(
	report.women.filter(over21),
	report.men.filter(over21)
	)
	}

	val generator = ReportGenerator(repository, withPeopleOver21)
	val reportWithPeopleOver21 = generator.create()

view raw ioc__dependency_injection.kt hosted with ❤ by GitHub

we can change the generator and have the filtering code being injected to it by constructor. The inversion is already happening, DI is simply used to provide the extra code.

SLAP: Single Level of Abstraction Principle

Even though this acronym is quite catchy, SLAP is the one principle that you don’t find many people talking about.

Single Level of Abstraction

In essence, the principle proposes that each block of code should not mix what the code does with how it does it. Another way of thinking about it is, whenever possible, the code should describe in steps the actions that will take and then each step can elaborate on that.

For example:

	class CreateTask(
	private val clock: Clock,
	private val localStorage: LocalStorage,
	private val observers: List<TaskObserver>
	) {

	fun invoke(description: String) {
	// normalize description
	val normalizedDescription = if (description.length > MAX_DESCRIPTION_LENGTH)
	description.substring(0, MAX_DESCRIPTION_LENGTH – 1) else
	description

	// create task
	val currentTime = clock.now()
	val initialStatus = Status.NotStarted
	val newTask = Task(normalizedDescription, initialStatus, currentTime)

	// save task
	localStorage.save(newTask)

	// notify
	val observingNotStarting = mutableListOf<TaskObserver>()
	for (i in 0..observers.size) {
	val taskObserver = observers[i]
	if (taskObserver.observedStatus == Status.NotStarted) {
	observingNotStarting.add(taskObserver)
	}
	}
	for (i in 0..observingNotStarting.size) {
	observingNotStarting[i].notify(newTask)
	}
	}
	}

view raw slap__many_levels.kt hosted with ❤ by GitHub

this class has one method that showcases both what it does and how it does it.

If we want to SLAP it we need to delegate the how of each step to its own method:

	class CreateTask(
	private val clock: Clock,
	private val localStorage: LocalStorage,
	private val observers: List<TaskObserver>
	) {

	fun invoke(description: String) {
	val newTask = createNewTask(description)
	localStorage.save(newTask)
	notifyAnyObservers(newTask)
	}

	private fun createNewTask(description: String): Task {
	val normalizedDescription = normalize(description)
	return Task(normalizedDescription, Status.NotStarted, clock.now())
	}

	private fun normalize(description: String): String {
	return if (description.length > MAX_DESCRIPTION_LENGTH)
	description.substring(0, MAX_DESCRIPTION_LENGTH – 1) else
	description
	}

	private fun notifyAnyObservers(newTask: Task) {
	observers
	.filter { taskObserver -> taskObserver.observedStatus == Status.NotStarted }
	.forEach { taskObserver -> taskObserver.notify(newTask) }
	}
	}

view raw slap__one_level.kt hosted with ❤ by GitHub

here the invoke method simply describes what will happen upon its invocation. A new task will be created, then saved and finally passed to any observers.
For knowing how each step gets implemented we need to drill down one level. For example, creating a new task requires us to normalize the provided description and then create the task. For knowing how the normalization gets implemented we yet again move one level deeper!

Conclusion

Keep hiding how something gets implemented in new methods until you can no longer avoid it!

Don’t force your objects to construct what they need

Let’s say we have an object that handles instances of Person. For example PeopleScreen:

	// Person.kt
	class Person(
	val name: String,
	val surname: String
	)

	// PeopleScreen.kt
	class PeopleScreen(
	private val people: List<Person>
	) {

	fun render() {
	people.forEachIndexed { index, person ->
	println("${index + 1}. ${person.name}, ${person.surname}")
	}
	}
	}

	// Usage:
	fun main() {
	val people = listOf(
	Person("Joe", "Dow"),
	Person("Jill", "Doe"),
	Person("Jack", "Black")
	)

	val screen = PeopleScreen(people)
	screen.render()
	}

view raw dont_force_construction__the_proper_way.kt hosted with ❤ by GitHub

PeopleScreen renders instances of Person so this should be the only format we provide to it. Let me explain.

Forced construction

There is a new flow that ends in opening PeopleScreen but all the information for the list of people are in a Map<String, String>. There is no reason to alter PeopleScreen in order to support this new format:

	// Person.kt
	class Person(
	val name: String,
	val surname: String
	)

	// PeopleScreen.kt
	class PeopleScreen(
	private val people: List<Person>
	) {

	constructor(people: Map<String,String>) : this(
	people.map { entry -> Person(entry.key, entry.value) }
	)

	fun render() {
	people.forEachIndexed { index, person ->
	println("${index + 1}. ${person.name}, ${person.surname}")
	}
	}
	}

	// Usage:
	fun main() {
	val people = mapOf(
	"Joe" to "Dow",
	"Jill" to "Doe",
	"Jack" to "Black"
	)

	val screen = PeopleScreen(people)
	screen.render()
	}

view raw dont_force_construction__the_forced_way.kt hosted with ❤ by GitHub

the new format is passed through an overloaded constructor but the same goes if we use a setter method

Why we shouldn’t do it

We could argue that by doing so we tie the object with each special format making the code hard to maintain and scale but the real reason is that we violate the SRP principle since PeopleScreen will have more than one reasons to change. One if something changes in the way we render and two if something changes in Person‘s construction.

What we should do

We should keep PeopleScreen only consuming Person and move all transformations to their own objects allowing a coordinator to transform and pass data around.