Why Kotlin Code Review Isn't About "I Would Have Written It Differently"?
\n\nCode review is a sacred ritual in any self-respecting team. But when it comes to Kotlin, many developers get stuck. "Everything is so concise here!", "Well, it's an expression-based language" — you think, scrolling through a pull request. And you'd be wrong.
\n\nKotlin is beautiful but treacherous. It gives you powerful tools: scope functions, sealed classes, coroutines. And just as many opportunities to shoot yourself in the foot. In this article, we'll break down what you really need to pay attention to during a Kotlin code review so your code doesn't turn into a "spaghetti monster" with nullable side effects.
\n\nThis article is not a syntax tutorial. It's a living checklist for a reviewer who wants to see not just working code, but a maintainable and safe product.
\n\n1. Null Safety: When "?" Is Not a Panacea
\n\nKotlin prides itself on its Null Safety. And rightly so. But during a code review, you must check not just for the presence of "!!", but the logic of working with nullable types.
\n\nWhat Should Definitely Raise a Red Flag?
\n\n- \n
- Uncontrolled use of "!!". If a developer uses
!!, they must explain in a comment why this value is guaranteed not to be null. If there's no explanation, it's a bug. \n - Chains of safe calls (?.) and elvis (?:). They often mask logical errors. Example: \n
// Problem: if user?.address?.city == null, \n// we silently return "Unknown" without logging\nval city = user?.address?.city ?: "Unknown"\n\n\nHow should it be done? If null is an exceptional situation, use requireNotNull or checkNotNull. If it's normal behavior, log it.
val city = user?.address?.city\n ?: run {\n logger.warn("User $userId has no city")\n "Unknown"\n }\n\n\nDuring a Kotlin code review, always ask: "What if this null is a bug that we are now silencing?"
\n\n2. Scope Functions: let, apply, run — Use Them Wisely
\n\nScope functions are the power of Kotlin. But in practice, I see them used like a hammer for every nail. Kotlin code checking should include the analysis: "Why is let used here and not run?"
Typical Pitfalls:
\n\n- \n
- let for null checks. Okay, but don't nest them: \n
// Awful\nuser?.let { u ->\n address?.let { a ->\n process(u, a)\n }\n}\n\n// Good\nif (user != null && address != null) {\n process(user, address)\n}\n\n\n- \n
- apply for mutating an object. This is its natural habitat. But if you use
applyinsidelet— stop. It smells like spaghetti. \n
Remember: with is for grouping calls, also is for side effects (logging), run is for computations within a context. Don't confuse them.
3. Immutability: var Is a Red Flag
\n\nKotlin advocates for val. And that's great. But during a Kotlin code review, I often see developers using var where val with copying would suffice.
// Bad\nvar result = mutableListOf<Int>()\nitems.forEach { result.add(it * 2) }\n\n// Good\nval result = items.map { it * 2 }\n\n\nThe rule is simple: if you see var, ask: "Why not val?" Most often, the answer will surprise you: "I didn't think about it."
The only exception is when mutation is truly necessary (e.g., in accumulation loops). But even then, it can often be replaced with fold or reduce.
4. Sealed Classes and when: Don't Forget About else
\n\nSealed classes are an elegant alterna