Improving code quality - Session 6: If I had eight hours to chop down a line...

Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.

This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.

If I had eight hours to chop down a line...

The following code retrieves the display name (displayName) if ContactModel is of type Person and in "friend" status.

fun ...(contact: ContactModel): ReturnValue? {
    val friendName = (contact as?
            ContactModel.Person)?.takeIf {
        it.isFriend
    }?.let { normalizeEmoji(it.displayName) } ?: return null

    // snip...
    // snip...
}

Note, let is a standard Kotlin function that behaves as follows in this code:

• If the return value of ?.takeIf is null: returns null without doing anything
• If the return value of ?.takeIf is non-null: calls normalizeEmoji(it.displayName) using the return value as it, and returns the result

Are there any improvements that could be made to this code?

Sharpening my axe first

The code above is made difficult to read by inappropriate line breaks. Even without altering the logic, changing the position of the line breaks can make it more readable.

    val friendName = (contact as? ContactModel.Person)
        ?.takeIf { it.isFriend }
        ?.let { normalizeEmoji(it.displayName) }
        ?: return null

Generally, it's good to break lines at places where "the meaning significantly changes". However, choosing the right place to break can be challenging. In such cases, translating the code into a natural language (such as English or Japanese) can be helpful. This can clarify where the significant breaks are.

The code above can be translated as follows:

If a contact is a Person and the person is a friend, take the name with normalizing; otherwise this returns null.

Here, inserting slashes where the meaning significantly changes, it would look like this:

If a contact is a Person / and the person is a friend, / take the name with normalizing; / otherwise this returns null.

And this corresponds to the improved code's line breaks.

On the other hand, inserting slashes corresponding to the initial example's line breaks would look like this:

If a contact is a / Person and the person is / a friend / , take the name with normalizing; otherwise this returns null.

Correctly placing line breaks can also facilitate more advanced refactoring. The first line's as? ContactModel.Person and the second line's .takeIf act as filters, with the fourth line's ?: return null corresponding to the outcome. This realization suggests that the third line's normalizeEmoji(it.displayName) could be moved outside the method chain.

    val friend = (contact as? ContactModel.Person)
        ?.takeIf { it.isFriend }
        ?: return null
    val friendName = normalizeEmoji(friend.displayName)

Various ways of cutting

Method chains / fallback chains

When using method chains with operators like . and safe call ?., or fallback chains with ?:, the structure and flow of the logic are often more important than the details of the code. In such cases, it's beneficial to place line breaks just before these operators.

    val ... = someCollection
        .filterIsInstance<SomeModel>()
        .filter { ... }
        .map { ... }
        .toSet()

    val ... = nullable?.value
        ?: fallback.value
        ?: another.fallback(value)

If the actual arguments become lengthy (including lambdas), using auxiliary functions or extension functions to shorten the arguments can help align the line breaks.

    val ... = nullable?.value
        ?: fallback.shortcut
        ?: another.fallback(value)
...
private val Fallback.shortcut: ...? get() =
    value.with(long.long.long.long.long.long.long.argument)

Operator precedence

In many cases, the precedence of operators corresponds to the strength of the connections in meaning. For example, breaking at == is clearer than breaking at + or -.

valueWithLongName1 - valueWithLongName2 ==
    valueWithLongName3 + valueWithLongName4

valueWithLongName1 -
    valueWithLongName2 == valueWithLongName3 +
    valueWithLongName4

When using (), the section enclosed in parentheses has a stronger connection.

valueWithLongName1 *
    (valueWithLongName2 + valueWithLongName3)

Elvis return ?: return

If a piece of code has effects that are not localized, it's advisable to emphasize those effects by carefully choosing the line break positions. Prime examples of this would be return and throw. When using return or throw, placing them on the left side of the code emphasizes their importance.

Therefore, it's a good practice to insert a line break just before ?: return, ?: throw, or ?: error(...) when using these constructs.

    val nonNullValue = some.nullable.value.with(parameter)
        ?: return someReturnValue

---

In summary: When breaking lines in code, be mindful of the divisions in meaning.

Keywords: line-break, code chunk, operator precedence