I was writing some code for KotlinPoet and I was writing a function that decides whether kotlin.Unit should be emitted as a return type (applications can choose to omit Unit as a return type but libraries should be explicit about what they’re returning for compatibility). Here’s a shell:

/**
 * Returns whether [Unit] should be emitted as the return type.
 *
 * [Unit] is emitted as return type on a function unless:
 *   - It's a constructor
 *   - It's a getter/setter on a property
 *   - It's an expression body
 */
private fun emitUnitReturnType(): Boolean {
}

We write code the way we think, so the logic for this function could be something like:

/**
 * Returns whether [Unit] should be emitted as the return type.
 *
 * [Unit] is emitted as return type on a function unless:
 *   - It's a constructor
 *   - It's a getter/setter on a property
 *   - It's an expression body
 */
private fun emitUnitReturnType(): Boolean {
  return !isConstructor && 
      (name != GETTER && name != SETTER) && 
      body.asExpressionBody() == null
}

This perfectly mirrors our thought process but this code is horrendous to review. I don’t know about you, but it will take me a minute to understand what’s happening here. If the kdoc wasn’t present, I’d be lost.

We can simplify this by pulling out a few variables:

/**
 * Returns whether [Unit] should be emitted as the return type.
 *
 * [Unit] is emitted as return type on a function unless:
 *   - It's a constructor
 *   - It's a getter/setter on a property
 *   - It's an expression body
 */
private fun emitUnitReturnType(): Boolean {
  val isGetterOrSetter = name == GETTER || name == SETTER
  val isExpressionBody = body.asExpressionBody() != null
  return !isConstructor && !isGetterOrSetter && !isExpressionBody
}

This is much better but software evolves. Maybe Kotlin changes their mind about when we should emit Unit as return type. In that case, we’d have to add another boolean to this combinatorial logic. If you work in feature code, I don’t have to remind you about ever-evolving and changing business conditions.

Here’s a variation that reads much better:

private fun emitUnitReturnType(): Boolean {
  if (isConstructor) {
    return false
  }
  if (name == GETTER || name == SETTER) {
    // Getter/setters don't emit return types
    return false
  }

  return body.asExpressionBody() == null
}

By deconstructing each condition and returning early, this code is a lot easier to read. I also omitted the docs because now the code is self documenting.

And what if you need to add more conditions? That’s easy for you as a programmer (just another if statement) and easy for your reviewer since they’re only evaluating one extra statement.

As a code reviewer, I find combined booleans to be taxing. Do your code reviewer (and future self) a favor and break them down into multiple statements.