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