コード品質向上のテクニック: 第 19 回（チャイルドロック）

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

この記事は、毎週木曜の定期連載 “Weekly Report” 共有の第 19 回です。Weekly Report については、第 1 回の記事を参照してください。

チャイルドロック

メッセージのデータモデルとして、MessageData という抽象クラスがあるとします。メッセージにはいくつかの種類があり、その種類を MessageData の子クラスを定義することで表現しているとしましょう。

ここで、MessageData の子クラス T のリスト List<T> を画面に表示するため、以下のような抽象クラス MessageListPresenter<T : MessageData> が定義されたとします。ただし、メッセージの表示ロジックはその種類で変わるため、MessageListPresenter.bind ではリストの表示ロジックが実装されていません。親クラスの bind ではヘッダーとフッターの表示だけが行われ、リストの表示は子クラスでオーバーライドして実装されることを期待しています。(Kotlin の open はオーバーライド可能という意味の修飾子です。)

abstract class MessageListPresenter<T : MessageData>(
    ...
) {
    private val headerView: ... = ...
    private val footerView: ... = ...

    open fun bind(messageList: List<T>) {
        updateHeader(messageList.size)
        updateFooter(messageList)

        // ここでは `messageList` の表示は実装されていない。
        // この関数をオーバーライドして `messageList` の表示を実装すること。
    }

    private fun updateHeader(messageCount: Int) {
        headerView.text = ...
        headerView....
    }

    private fun updateFooter(messageList: List<T>) {
        footerView....
    }
}

以下の SomeSpecificMessageListPresenter は、MessageListPresenter の実装例です。タイプパラメータとして SomeSpecificMessageData を指定し、そのクラス固有のリスト表示ロジックを bind 中に定義しています。

class SomeSpecificMessageListPresenter(
    ...
) : MessageListPresenter<SomeSpecificMessageData>(
    ...
) {
    override fun bind(messageList: List<SomeSpecificMessageData>) {
        super.bind(messageList)

        ... // `messageList` を表示
    }
}

このコードになにか問題点はありますか？

子クラスにいたずらさせない

多くの場合、super を明示的に呼び出す必要があるということは、オーバーライド可能な関数の範囲が広すぎる ことを意味します (例外については後述)。このコードでは、オーバーライド可能な関数の範囲が広すぎることが原因で、以下のような実装ミスを引き起こしやすくなっています。

super の呼び出し忘れ: すべての子クラスは、ヘッダーやフッターの更新のために super.bind を呼び出すことを期待しています。しかし、super.bind を呼び忘れた場合にヘッダーやフッターが更新されないというバグが発生します。さらに、その時に特に何のエラーにも起きないため、バグが見落とされやすいという問題もあります。
bind のオーバーライド忘れ: 継承時にオーバーライドが必要という制約は、インラインコメントでしか説明されていません。オーバーライドすべき関数が他にもあるという状況では、オーバーライドを忘れるというバグも発生しやすくなります。オーバーライドを強制させるためにも abstract 修飾子を使いたくなりますが、ヘッダーやフッターの更新のロジックが混ざり込んでいると、それも難しいです。
期待していない bind の実装: インラインコメントに書かれている通り、オーバーライドされた bind では、メッセージリストの表示の実装だけが行われることを期待しています。しかし、実際に bind を呼び出すと、ヘッダーやフッターの表示も行われます。つまり、オーバーライドの責任範囲と関数の責任範囲が一致していないと言えるでしょう。これにより、オーバーライドの責任範囲が勘違いされやすくなり、結果として責任範囲外のコードが子クラスに実装されかねません。

この問題を解決するためには、bind 関数を open にするのではなく、リストを更新をする関数 updateMessageList を分け、それを abstract として抽象メソッド（や純粋仮想関数）にするとよいでしょう。

abstract class MessageListPresenter<T : MessageData>(
    ...
) {
    ...

    fun bind(messageList: List<T>) { // `open` ではない
        updateHeader(messageList.size)
        updateFooter(messageList)
        updateMessageList(messageList)
    }

    protected abstract fun updateMessageList(messageList: List<T>)

    ...
}

子クラスは以下のようになります。

class SomeSpecificMessageListPresenter(
    ...
) : MessageListPresenter<SomeSpecificMessageData>(
    ...
) {
    override fun updateMessageList(messageList: List<SomeSpecificMessageData>) {
        ... // `messageList` を表示
    }
}

このようにすることで、「ヘッダーやフッターとメッセージリストを更新する」という bind の流れを変更不能にしつつ、メッセージリストの実装を子クラスごとに変えることができます。このように、子クラスが変更できる範囲を限定することは、コードを頑健にする上で重要です。

super は super すぎる

同じような問題を起こさないためにも、以下のような状況を 避ける とよいでしょう。

オーバーライドされた親クラスの関数を呼び出している (super を明示的に使っている)。ただし、ライフサイクルに関係する関数 (コンストラクタやデストラクタ等) と、プラットフォームやライブラリの制約で必要な場合 (Android における Activity.onCreate 等) は例外。
「関数の流れ」が子クラスで共通しているのにもかかわらず、各子クラス内でそれを実装している。

これらの状況に当てはまる場合は、オーバーライド可能な範囲を制限することを試みてください。

余談: C++ の private virtual

C++ では private virtual な関数を定義できます。この関数は親クラスでしか呼び出せませんが、その呼び出しは動的にディスパッチされます。private virtual な関数を使うことで、全体的な流れ (callingPrivateVirtual) の変更を実質的に防ぎつつ、ロジック (privateVirtual) を子クラスごとに変えることができます。

class Base
{
public:
    void callingPrivateVirtual()
    {
        ... // 他のロジック
        privateVirtual(); // `Base` でだけ呼び出せる
    }

private:
    virtual void privateVirtual() {}
};

class Derived : public Base
{
private:
    void privateVirtual() override
    {
        // `privateVirtual` の実装の変更は可能。
    }
};

一言まとめ: オーバーライド可能な範囲はできる限り制限する。

キーワード: override, super, inheritance