Kotlin Conversions — 4 Pitfalls You Should Look out For
If you are an Android developer working on a codebase that is more than a couple of years old, you are probably converting code to Kotlin right now. At Blinkist, we too are big fans of Kotlin and we have therefore been converting code to Kotlin quite a lot:
Most of the time this has been a smooth and rewarding process, but we still hit small and large bumps in the road. In this post I am going to describe our workflow for Kotlin conversion and highlight the main pitfalls that we have encountered and that you should look out for.
How we convert
We usually use commit-by-commit reviewable pull requests at Blinkist and they are a great fit for Kotlin conversion. First, we apply the automatic conversion of Android Studio (cmd-shift-alt-k) and immediately commit the results. We don’t fix formatting, compile errors or global warming at this step. This way a reviewer can differentiate between the (reasonably safe) automatic conversion and any additional changes that are added by us.
Once we have commited the automatic conversion we can start with making things nice, get rid of some nullability, introduce single-expression functions, change variable names and… hold on, the interaction of this class with that other class does not make sense, we should fix that too! Which brings us to pitfall number one:
Pitfall 1 — Drive-by-Refactorings
Imagine this: while converting code to Kotlin you find a code smell. And it’s a big one. You can not believe how the the person that wrote this code in the past (which was probably you) could have done this. But it seems easy to fix, so you are going to quickly do that. Easy.
Don’t. You are not here right now to bring world peace to your codebase. You may not have the full context and you are going to confuse your reviewers if you do both a Kotlin conversion and structural change in the same PR. Get the PR merged and come back later to finish the job. Once more classes are converted to Kotlin, that refactoring might be a piece of cake!
Now imagine being on the other side, reviewing a Kotlin conversion PR. Things look great and you are almost ready to approve it, but then you spot a code smell. It’s a bit tricky to fix and it was part of the original Java code, but hey, we should apply the boy scout rule and leave the code better than we found it, right? You request changes, this needs to be fixed now!
Don’t. If the code is not worse than it was before and it is not something that can safely be fixed by sprinkling a bit of Kotlin sugar on it, make a comment, but approve the PR. Otherwise you might push the person that opened the PR to make risky changes. Requesting to fix everything along the way may also discourage members on your team from attempting future Kotlin conversions. Safe baby steps are better here than not moving at all.
Pitfall 2 — Unexpected Nullability
Automatic conversions are usually a good first step and reasonably safe, but they may still contain pitfalls. If you are receiving a callback from a Java interface with parameters that are not annotated as nullable, these are assumed to be non-null by the automatic conversion, while in reality they might be null. For us, this happened with onActivityResult() and it led to crashes in production when the returned data bundle was null unexpectedly. Bottom line, double check that any non-null value that is passed to you from a Java class is really never null or treat it as nullable just to be safe.
Pitfall 3— lateinit overuse
lateinit is a great language feature of Kotlin, especially for classes extending Android framework components like Activities, where a lot of fields are only initialized when lifecycle methods like onCreate() are run. lateinit therefore goes a long way to removing undesired double bangs after automatic conversions. But it can also be dangerous, as it disables the compile time nullability check on a field.
To illustrate, let’s say that the Java class you want to convert expects a mandatory listener to be passed to it right after the class is instantiated, which it stores in a field, i.e setListener(someListener). This is probably not a good design decision, but maybe there are limitations, like the Android framework, that forced this in the past. When you auto-convert this class to Kotlin, that listener field will be annotated as nullable and its usages will be branded with the double bangs of shame.
This is not pretty and should be changed, but not by making the field lateinit. By doing so, you would not be fixing the smell, but just hiding it under the rug. If you are VERY sure that you can apply a proper fix with no risk of breakage, do so now. But you are going to run into Pitfall 1 more likely than not while doing so, so the best course of action might be to just come back later and apply a proper fix.
Pitfall 4— val Madness
val's in Kotlin are extremely versatile and powerful, but they can be tricky to get right, especially on Android.
When initializing a val field on instance creation of a class, make sure that the resources that you are trying to access are actually available. If you are accessing a lateinit field while doing so, its resource might not be set yet and you might get an exception. Also, if you are accessing any Android framework function like getString() on a Fragment for example, the lifecycle methods will not have run yet and calling that function at this point will throw an exception.
Another popular thing to do during Kotlin conversions is turning getters into vals. This is usually a good thing because it reduces verbosity, but there is another pitfall here: a getter accesses its data source every time it is called, while a val by default retrieves the value and stores it.
If the data source is static and does not change, this is not an issue. But if the data source is a database repository for example, a getter would access the database and fetch the data every time the getter is called. This is very different with a val, which, when accessed, would simply return the value that was fetched during initialization. The database is not queried and the value might become outdated.
This can be fixed by adding a custom getter to the val that queries the data source every time the val is accessed, but this is easy to forget. This is especially true if you are extending an abstract class, where you don’t know if that base class expects the data source to be queried every time the field is accessed. This caused a major bug for us that was luckily caught during regression testing.
Generally speaking, if you are using abstract classes in your codebase, try to convert them first before converting their implementations. The automatic conversion simply does a better job if it is done this way and it creates more possibilities to use Kotlin exclusive features. The same applies to interfaces.
Wrap Up
Try not to do too much at the same time when converting classes in your codebase to Kotlin or you may end up creating bugs. Commit-by-commit reviewable pull requests help your reviewers get a clear picture of your changes. Be careful with refactoring during conversions and resist the urge to use all of Kotlin’s amazing features at once. That being said, converting code to Kotlin opens up countless possibilities for making your code more concise, flexible and robust, so go for it!
