LINEヤフー Tech Blog

LINEヤフー株式会社のサービスを支える、技術・開発文化を発信しています。

コード品質向上のテクニック:第36回 「トートロジーはトートロジー」はトートロジー

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

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

「トートロジーはトートロジー」はトートロジー

とあるグループウェアを実装していることを仮定しましょう。このグループウェアには、タスクを管理する機能と、ユーザ同士でメッセージを送り合う機能があるとします。また、何かタスクに更新がある時や、新しいメッセージを受信した時は、通知ドット (notificationDot) でお知らせする機能もあるとします。この通知ドットの実装は以下の通りです。ただし、(any の計算コストも含め) このロジックとデータモデルには問題がないとします。

        notificationDot.isVisible = channelModels.any(ChannelModel::hasUnreadMessage) ||
            directMessageRoomModels.any(DirectMessageRoomModel::hasUnreadMessage) ||
            taskListStatus.hasNewlyAssignedTask ||
            taskListStatus.hasNewlyReopenedTask ||
            taskListStatus.hasUpdateOnAssignedTask

        ...

isVisible の計算ロジックは少し長く、見通しがあまり良くないです。そこで、この計算ロジックを以下のようにプライベート関数 shouldShowNotificationDot として抽出しました。

        notificationDot.isVisible = shouldShowNotificationDot(
            channelModels,
            directMessageRoomModels,
            taskListStatus
        )

        ...
    }

    private fun shouldShowNotificationDot(
        channelModels: List<ChannelModel>,
        directMessageRoomModels: List<DirectMessageRoomModel>,
        taskListStatus: TaskListStatus
    ): Boolean = channelModels.any(ChannelModel::hasUnreadMessage) ||
        directMessageRoomModels.any(DirectMessageRoomModel::hasUnreadMessage) ||
        taskListStatus.hasNewlyAssignedTask ||
        taskListStatus.hasNewlyReopenedTask ||
        taskListStatus.hasUpdateOnAssignedTask

このようにして、呼び出し元を単純にすることができました。しかし、これは読みやすさの観点で最善とは言いにくいです。より良い方法何かあるでしょうか?

意味のあるフレーズにする

notificationDot.isVisible = shouldShowNotificationDot( の行を自然言語として読み下すと、「notificationDot を表示するべきときに、notificationDot を表示する」となります。これでは、コードを読む人に与える情報は限定的です。

このコードが何をするのかを知りたい場合は、= の時点で読むのを中断しても構いません。notificationDot.isVisible = を見ただけで、「このコードは notificationDot の表示状態を更新する」ということを理解できます。裏を返せば、= の右側 (や継続行) を読みたいということは、「どういう条件のときに notificationDot が表示されるか」を知りたいのだと言えます。しかし、実際に = の右側と継続行を読んでも、「notificationDot の表示状態は channelModels などに依存する」としかわかりません。実際の条件を理解するためには、shouldShowNotificationDot の定義を読む必要があります。しかしこれでは、= の右側と継続行の持つ情報量はあまりないと言えるでしょう。

コードから多くの情報を読み取れるようにする (コードの情報量を増やす) ためにも、抽出したコードに与える名前を少し詳細にする ことは良い選択肢です。以下のコードでは、 notificationDot.isVisible = で読むのを中断しても、「何をするのか」を知ることができます。一方で、条件の概要を理解する必要があるならば、...isVisible = の後ろを読めば十分で、「未読メッセージか未読タスクがある」ことだと分かります。

        notificationDot.isVisible =
            hasAnyUnreadMessage(channelModels, directMessageRoomModels) ||
                hasAnyUnreadTask(taskListStatus)
    }

    private fun hasAnyUnreadMessage(
        channelModels: List<ChannelModel>,
        directMessageRoomModels: List<DirectMessageRoomModel>,
    ) = channelModels.any(ChannelModel::hasUnreadMessage) ||
            directMessageRoomModels.any(DirectMessageRoomModel::hasUnreadMessage)

    private fun hasAnyUnreadTask(taskListStatus: TaskListStatus): Boolean =
        taskListStatus.hasNewlyAssignedTask ||
            taskListStatus.hasNewlyReopenedTask ||
            taskListStatus.hasUpdateOnAssignedTask

この例では右辺値で || の演算を行っていますが、このような計算や操作は必ずしも必須ではありません。単純な変数の代入だったとしても、同様の考え方を適用できます。以下の 2 つの代入を見比べてください。

// having little information
notificationDot.isVisible = shouldShowDot

// having more information
notificationDot.isVisible = hasUnreadMessage

もちろん、この考え方は代入だけではなく、様々なケースに応用できます。以下のコードは、条件分岐に適用した例です。

// having little information
if (shouldShowDot) {
    notificationDot.show()
}

// having more information
if (hasUnreadMessage) {
    notificationDot.show()
}

ただし、この「抽出したコードに与える名前を少し詳細にする」という考え方は、常に適用するべきという訳ではありません。API やライブラリを作る場合や、コードをモジュール化・レイヤ分けする場合など、依存先の詳細を隠蔽するべき場合は、「依存元が何をするべきか」に基づいて名前をつけることもあります。他にも、汎用的な UI 要素のイベント処理インターフェース (例: onClicked) など、動作の詳細が決まっていない場合も同様です。この場合、イベント処理のコードとしては、「クリックが発生したときに onClicked を呼び出す」というコードにせざるを得ません。少し詳細な名前を与えるというアイディアは、あくまでも選択肢の 1 つに過ぎないということに留意してください。

一言まとめ

コードを抽出したときは、情報量が増えるような名前を与えることを考慮する。

キーワード: naming, abstraction, extraction

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

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