Conditional statements are a code smell

Yes, conditional statements are considered code smell if they are used extensively all over the project. alrighty, let us jump straight to the example to understand why.

let’s assume we are making a network request to different news sources to get the top headlines from each source.

fun getHeadlines(source: String) {
    when (source) {
      "BBC" -> fetchNewsFromBBC()
      "CNN" -> fetchNewsFromCNN()
      "Guardian" -> fetchNewsFromGuardian()
    }
}

The problem with the above code is that it violates both Single-responsibility principle and Open–closed principle principle. I would highly suggest reading this if you are not familiar with SOLID principles. Let’s understand the problems one by one.

Problems

  1. getHeadlines is clearly handling more than one responsibility, currently, it’s handling 3 responsibilities.
  2. If we want to add a new source in the future we have to touch this piece of logic which we don’t want to do. We want this method to be future proof so that even if we need to add few more sources in the future we don’t even have to do anything in this method.


How can we improve this

We can use one of the OOP concept Polymorphism to solve this to make our code cleaner. Let’s see how

interface News {
   fun fetchNews()
}
class BBCNews(): News {
    override fun fetchNews() {
       return fetchNewsFromBBC()
    }
}
class CNNNews(): News {
    override fun fetchNews() {
       return fetchNewsFromCNN()
    }
}
class GuardianNews(): News {
    override fun fetchNews() {
       return fetchNewsFromGuardian()
    }
}

This is how getHeadlines looks like now

fun getHeadlines(newsSource: News) {
    newsSource.fetchNews()
}

we can now call getHeadlines function as shown below

getHeadlines(BBCNews()) // for BBC news
getHeadlines(CNNNews()) // for CNN news
getHeadlines(GuardianNews()) // for Guardian news


What if i want to add new source?

Got new source?. Just do this

class CNBCNews(): News {
    override fun fetchNews() {
       return fetchNewsFromCNBC()
    }
}
getHeadlines(CNBCNews()) // for CNBC news


Now getHeadlines method is much cleaner to read and is clearly handling single responsibility. Also, we don’t need to touch on this method if we want to add a new source in the future. How awesome is that!

so, whenever you see conditional statements which especially operate on type of the object then consider replacing those with OOP concepts and design patterns


Untill next time