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.
The only truth
Suppose a class called UserModel is defined as the data model for user accounts.
class UserModel(
val userId: UserId
val accountType: AccountType,
val displayName: String,
val profileImageUri: Uri,
val onlineStatus: OnlineStatus,
val moodMessage: String
)Among these properties, accountType, displayName, and profileImageUri are widely used within the application, and the timing of their value updates is limited. On the other hand, onlineStatus and moodMessage are used only in specific features (e.g., user detail screen) and are frequently updated.
When there is a variation in the lifecycle of values and the scope of code that depends on those values, separating the data model can lead to robust and clear code. In this case, the direction of dependencies should be as follows:
- Feature-specific code depends on generic code
- Values with a short lifecycle depend on values with a long lifecycle
(For more on the direction of dependencies, refer to https://speakerdeck.com/munetoshi/code-readability-session-7-ver-2-ja?slide=4 slide=35 and the "6-3 Dependency Direction" section in Guidelines for Readable Code.)
In this case, it would be good to separate the three properties accountType, displayName, profileImageUri from the two properties onlineStatus, moodMessage, with the latter depending on the former.
Therefore, a developer applied "aggregation" and rewrote UserModel as follows. However, there are improvements to be made in this code. What are they?
class UserState(
val userId: UserId
val onlineStatus: OnlineStatus,
val moodMessage: String,
val accountModel: AccountModel
)
class AccountModel(
val userId: UserId,
val accountType: AccountType,
val displayName: String,
val profileImageUri: Uri
)False truth
The issue with this code is that UserState holds an AccountModel, yet both UserState and AccountModel have their own userId. This configuration allows for the easy creation of "invalid" instances, as shown below:
UserState(
UserId("userA")
...,
...,
AccountModel(
UserId("userB"),
...
)
)When applying aggregation or composition, it is preferable to avoid value duplication. If UserState holds an instance of AccountModel, UserState should not have an externally provided userId but should use AccountModel.userId. You can simply remove UserState.userId or make UserState.userId determined by AccountModel.userId.
// Option 1: Delete `userId`
class UserState(
val onlineStatus: OnlineStatus,
val moodMessage: String,
val accountModel: AccountModel
)
// Option 2: Decide `userId` by `AccountModel`
class UserState(
val onlineStatus: OnlineStatus,
val moodMessage: String,
val accountModel: AccountModel
) {
val userId: UserId get() = accountModel.userId
// or, `val userId: UserId = accountModel.userId` if it's immutable.
}Such value duplication can occur not only due to dependency and non-dependency relationships but also between properties. The possibility of invalid value combinations can be eliminated with factory functions or failable initializers. The factory function of in the following ProfileModel creates an instance by combining AccountModel and UserGroupRelation, but returns null if the userId does not match.
class AccountModel(
val userId: UserId,
val displayName: String
)
class UserGroupRelation(
val userId: UserId,
val groups: Set<GroupId>
)
class ProfileModel private constructor(
val userId: UserId,
val displayName: String,
val groups: Set<GroupId>
) {
companion object {
fun of(
accountModel: AccountModel,
userGroupRelation: UserGroupRelation
): ProfileModel? {
if (accountModel.userId != userGroupRelation.userId) {
return null
}
return ProfileModel(
accountModel.userId,
accountModel.displayName
userGroupRelation.groups
)
}
}
}In a nutshell
When applying aggregation or composition, be cautious of invalid states due to value duplication.
Keywords: aggregation, value domain, property duplication