コード品質向上のテクニック：第50回たった一つの真実

こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。

この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 50 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。（Weekly Report の詳細については、過去の記事一覧を参照してください）

たった一つの真実

ユーザアカウントのデータモデルとして、UserModel というクラスが定義されているとします。

class UserModel(
    val userId: UserId
    val accountType: AccountType,
    val displayName: String,
    val profileImageUri: Uri,
    val onlineStatus: OnlineStatus,
    val moodMessage: String
)

これらのプロパティのうち、accountType と displayName、profileImageUri はアプリケーション内で広く使われており、かつ値の更新のタイミングは限定されているとします。一方で onlineStatus と modeMessage については、特定の機能 (例: ユーザ詳細画面) でしか使われておらず、かつ、頻繁に値が更新されるとします。

値のライフサイクルや、その値に依存するコードのスコープにばらつきがある場合は、データモデルを分離することで頑健で見通しの良いコードにできる可能性があります。このとき、依存の方向は以下のようにすると良いでしょう。

機能限定のコードが汎用的なコードに依存する
短いライフサイクルの値が長いライフサイクルの値に依存する

(依存の方向については、https://speakerdeck.com/munetoshi/code-readability-session-7-ver-2-ja?slide=4 から slide=35 や、読みやすいコードのガイドラインの「6-3 依存の方向」も併せて参照してください。)

今回の場合は accountType、displayName、profileImageUri の 3 つのプロパティと onlineStatus、modeMessage の 2 つのプロパティで分け、後者が前者に依存する形式にするのが良いでしょう。

そこで、ある開発者が「集約」を適用する形で UserModel を以下のように書き換えました。しかし、このコードには改善点があります。それは何でしょうか？

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
)

偽の真

このコードの問題点は、UserState が AccountModel を持つのにもかかわらず、UserState と AccountModel の両方が独自の userId を持っている点にあります。このような構成では、以下のように「不正な」インスタンスを簡単に作れてしまいます。

UserState(
    UserId("userA")
    ...,
    ...,
    AccountModel(
        UserId("userB"),
        ...
    )
)

集約やコンポジションを適用する場合は、値の重複を避ける のが好ましいです。UserState が AccountModel のインスタンスを持つならば、UserState は外部から与えられた userId を持つのではなく、AccountModel.userId を利用するべきです。単純に UserState.userId を削除しても良いですし、UserState.userId が 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.
}

このような値の重複は、依存・非依存の関係によってのみ発生する訳ではなく、プロパティ間でも発生する可能性があります。不正な値の組み合わせの可能性は、ファクトリ関数や failable initializer で排除できると良いでしょう。以下の ProfileModel のファクトリ関数 of は、AccountModel と UserGroupRelation を組み合わせてインスタンスを作成していますが、userId が一致しない場合は null を返却しています。

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
            )
        }
    }
}

一言まとめ

集約やコンポジションを適用するときは、値の重複による不正な状態に気をつける。

キーワード: aggregation, value domain, property duplication

コード品質向上のテクニックの他の記事を読む

コード品質向上のテクニックの記事一覧