Code review in Kotlin: what to check and how to give feedback

Code review in Kotlin: what to check and how to give feedback

Code review is one of the most effective tools for maintaining code quality, sharing knowledge within a team, and preventing bugs. However, simply "glancing over" a pull request is not enough. In the Kotlin language, with its rich type system, coroutines, and functional capabilities, it is important to pay attention to specific points. In this article, we will break down what exactly to look for when reviewing Kotlin code, and how to formulate comments so that they are beneficial rather than offensive.

1. Basic principles of constructive feedback

Before moving on to technical details, let's recall a few rules that make code review useful for all participants.

Focus on the code, not the author

Instead of "You wrote bad code," say "This method can be simplified using let." Specificity and a neutral tone reduce tension.

Categorization

Mark comments as:

• Blocking — must be fixed (logic error, resource leak).
• Nitpick — minor quibbles (style, formatting).
• Question — clarification (intent is unclear).

Explain the "why"

It's not enough to say "use filterNotNull()." Add: "This will eliminate an unnecessary loop and make the code self-documenting." This way the author learns, not just fixes.

2. Null safety and types: Kotlin's main allies

Kotlin is designed to avoid NullPointerException at compile time. Check that this protection is not compromised.

Avoid unnecessary !! (not-null assertion)

The !! operator is a cry for help. Its use should be the exception, not the rule.

// Bad: risk of NPEfun process(user: User?) {    val name = user!!.name}

// Good: safe callfun process(user: User?) {    val name = user?.name ?: "Guest"}

What to check: if you see !!, ask the author if it can be replaced with ?.let, ?: (elvis), or requireNotNull().

Correct use of nullable types

Not all parameters need to be nullable. If a function always expects a value, declare the type without a question mark.

// Debatable: null means nothingfun log(message: String?) {    println(message ?: "null")}

// Better: explicit requirementfun log(message: String) {    println(message)}

3. Coroutines and threads: async without pain

Kotlin Coroutines are a powerful tool, but they are easy to misuse. During review, pay attention to the following points.

Correct dispatcher for tasks

Do not run long operations (network, disk) on Dispatchers.Main.

// Bad: blocks the UIGlobalScope.launch(Dispatchers.Main) {    val data = fetchFromNetwork() // hangs}

// Good: context switchingviewModelScope.launch {    val data = withContext(Dispatchers.IO) {        fetchFromNetwork()    }    updateUI(data)}

Coroutine leaks and structured concurrency

Use viewModelScope, lifecycleScope, or coroutineScope instead of GlobalScope. The latter lives as long as the application lives and easily leads to leaks.

// Dangerous: leakGlobalScope.launch { /* ... */ }

// Safe: tied to the lifecyclelifecycleScope.launch { /* ... */ }

4. Collections and functional style: conciseness and readability

Kotlin offers a rich library for working with collections. Use it instead of explicit loops.

Chains instead of mutations

Avoid manually adding to a list if you can collect the result using a chain of operations.