LY Corporation Tech Blog

We are promoting the technology and development culture that supports the services of LY Corporation and LY Corporation Group (LINE Plus, LINE Taiwan and LINE Vietnam).

This post is also available in the following languages. Japanese

Improving code quality - Session 7: A seat for a new member

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.

A seat for a new member

Consider a model class representing a "user" as follows.

class UserData(
    val id: UserId,
    val name: String,
    val profileImageUri: String,
    ...
)

The property name represents the user's "login name". To accommodate requests to not disclose login names to other users, we plan to allow users to register a "nickname" as a public alias. As a result, UserData has been modified as follows (using null for unregistered nicknames is agreed upon and isn't a problem).

class UserData(
    val id: UserId,
    val name: String,
    val profileImageUri: String,
    ...
    /**
     * ...
     * If `nickname` is explicitly `null`, it means no nickname is registered.
     */
    val nickname: String?
)

Furthermore, the UI displaying the "user name" is implemented as follows.

userNameView.text = userData.nickname
    ?: userData.name

Are there any issues with these excerpts of code?

Don't put a seat on top of another seat

Adding new properties or methods alone may require renaming existing properties or methods. In the above code, the property name UserData.name should have been changed.

If we implement a UI nicknameView that only displays the nickname without showing the login name, there is a bug where the login name is used instead of the nickname, but it isn't immediately obvious.

val nickname = userData.name
nicknameView.isVisible = nickname.isNotEmpty()
nicknameView.text = nickname

It's hard to spot the bug at first glance because "nickname" seems like a type of "name". There's no actual inclusion relationship between the values of UserData.name and UserData.nickname, but the property names appear to include one. To avoid confusion, the name of UserData.name should be changed when UserData.nickname is added. Here, the property name name has been changed to loginName.

class UserData(
    val id: UserId,
    /** ... */
    val loginName: String,
    /** ... */
    val nickname: String?,
    val profileImageUri: String,
    ...
) {
    // Also, we may put property selection logic
    /** ... */
    val displayName: String
        get() = nickName ?: loginName
}

Thus, you can clearly distinguish between loginName and nickname. This code also explicitly prioritizes the nickname over the login name for the display name displayName.

Similar cases

Similar situations can occur in various cases.

Case 1

  • Existing element: Data stored locally fooData
  • New element: Data obtained via the network fooRemoteData
  • Rename suggestion: fooData -> fooLocalData

Case 2

  • Existing element: Creation timestamp timestamp
  • New element: Last update timestamp lastUpdateTimestamp
  • Rename suggestion: timestamp -> creationTimestamp

Case 3

  • Existing element: External link that can be added to a user's profile url
  • New element: Link to a user's profile image profileImageUrl
  • Rename suggestion: url -> externalLinkUrl

In summary: When adding new elements, consider renaming existing code.

Keywords: naming, new element, inclusive relation