Common Code-Smells in Kotlin
Using ‘it’ everywhere
Arguments in single argument lambdas are implicitly named it in Kotlin. This is helpful because it’s(pun intended) short and using it makes more sense than using $, $0 or whatever default names most other languages use. But it’s use should be restricted to really trivial cases and we should prefer giving explicit argument names. In most cases, optimizing code readability is better than reducing the number of keystrokes one has to type.
Remember that the number of times code is read is always greater than the number of times it is written.
I’ll be presenting to you, two snippets:
Try skimming through both of them quickly. Which one made sense more easily? For me, the second snippet was easier to understand. The reason is that in the first snippet, you’ve to carry a lot of context in your mind while in the second one, the argument names help you regain the context as move you down the call chain.
And trust me, this is a really simple case in which there isn’t going to be that much difference in readability. As you work on an app, you frequently write code with more logic, using different methods you’ve define elsewhere and a greater call chain length. And if you use RxJava, all this becomes even more important as you chain operator after operator.
Using vars instead of vals
This one’s pretty short and obvious. But it’s still easy to find usages of vars where vals are more appropriate in a lot of code-bases. You should definitely check out the benefits of immutability. There’s a reason that Kotlin makes it equally easy to declare a var and a val. Compare that with Java where you’re required to type a whole extra keyword(final) for the same.
Avoiding !! everywhere blindly
It’s pretty common to see people use the Elvis operator(?:) everywhere to avoid using !!. It’s to be expected, right? One of Kotlin’s features is avoiding NullPointerExceptions and !! can result in one. If we go by this line of thought alone, it makes complete sense to banish !! from our code.
Alas, that’s not the entire story. Suppose I’m working on the employee management software of a company where an employee can optionally be assigned a mentor. The mentor should also be an employee. This can be represented as following:
data class Employee(val id: Long, val mentor: Employee?)Mentor is null if the employee hasn’t been assigned a mentor.
Let’s say I’ve to get the ids of all the mentors. Given below are two functionally equivalent snippets of code:
Kotlin(as of time of writing) can’t infer that employee.mentor can’t be null in map. Since employee.mentor being null is impossible, both of the snippets work exactly the same in all situations. So, any differences between the two are purely subjective.
If you religiously avoid !! in your code-base, you might already be leaning towards the second snippet. Most linters also lean towards the same. But is it really better?
The first snippet communicates that in map, employee.mentor can’t be null. If it’s null, we’re dealing with an illegal state and we should crash. The second snippet communicates that employee.mentor may be null in map and if that’s the case, adding 0 to our list of mentor ids is totally fine.
IMHO the first snippet communicates our intent better.
Our code should not only instruct the computer but should also reflect our intent.
This is an obviously contrived example. In fact, I’ll be giving a snippet which avoids both !! and ?: in the next heading ;) But hopefully, it gets my point across.
I would like to stress that I’m not advocating for using !!s. It is important to avoid app-crashes. At the end of the day, we make apps for our users. And app-crashes quickly ruin any user’s experience. But sometimes, if our app reaches an illegal state, the only sensible thing to do is to crash. Trying to avoid app crashes can sometimes lead to even worse user experience because once your app reaches an illegal state, there’s no telling what other illegal states it’ll reach. Also, if you use a crash-reporting service(which you should), you’ll be informed of the issue if the app crashes and you’ll be able to fix the issue sooner.
Not using the standard library’s functions
If, while looking at the above mentioned snippets of code, you thought: “I can do this better”, you may already be familiar with the content under this heading. Still try reading what I’ve to say. If you didn’t think that way, don’t worry. I was also in the same boat a really short time ago.
Let me present you another code snippet:
val mentorIds = employees.mapNotNull{ employee -> employee.mentor?.id }This does the same thing as the old snippets. The only difference is that the code is more concise, readable and totally free of any code smells(!! or ?:).
Kotlin is full of such utilities. Most people don’t take out the time to go further than filter, map and friends. Trust me, a big part of using a language is knowing and using it’s utilities. And Kotlin provides a LOT of utilities. Spend time exploring them. I can assure you that you’ll definitely feel your code become better and better over time.
Overusing Kotlin expressions
This one was recently covered in this excellent post by Bartek Lipinsky. On a broad level, what he’s trying to tell us is that nesting expressions within expressions can make us lose context about what we’re trying to achieve. I recommend reading his linked post to understand this point.
That’s all for this post. Please let me know of any suggestions or other code-smells you’re familiar with in the comments :)
