LINEヤフー Tech Blog

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

コード品質向上のテクニック:第61回 悪いな、この関数1人用なんだ

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

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

悪いな、この関数 1 人用なんだ

以下の calculateSomeFormula は、長時間かかる計算の結果を返す関数です。この関数では、複数の計算を並行して行うため、スレッドセーフであることを前提とします。(Kotlin で並行性制御をしたい場合はコルーチンを使うのが普通ですが、ここでは呼び出し側の言語やプラットフォーム制限により、コルーチンを使用できないとします。)

class FormulaCalculator {
    fun calculateSomeFormula(x: Int): Int {
        ... // Takes long time to calculate
    }
}

さらに、計算中であるかどうかを UI で表示したいとしましょう。以下のコードでは、isCalculating というプロパティを追加して、それを実現しようとしています。

class FormulaCalculator {
    val isCalculating: Boolean get() = calculationState.get()
    private val calculationState: AtomicBoolean = AtomicBoolean()
    
    fun calculateSomeFormula(x: Int): Int {
        calculationState.set(true)
        return try {
            ... // Takes long time to calculate
        } finally {
            calculationState.set(false)
        }
    }
}

このコードに、何か問題はありますか?

状態の又貸し

isCalculating は正しく「計算中」の状態を示すことができていません。calculateSomeFormula(x = 42)calculateSomeFormula(x = 43) を同時に呼び出したことを想定しましょう。このとき、以下の順番で実行される可能性があります。

  1. x = 42calculateSomeFormula が呼び出される
  2. x = 42calculationStatetrue になる
  3. x = 43calculateSomeFormula が呼び出される
  4. x = 43calculationState の値が更新されるが、true が維持される
  5. x = 42 の計算が完了する
  6. x = 42calculationStatefalse になる

6 が完了した時点では、x = 43 の計算が進行中であるにもかかわらず、isCalculatingfalse になってしまっています。

この問題は、関数呼び出し状態を、単一のレシーバの状態として扱っている ために発生しています。レシーバ 1 つに対して複数回の関数呼び出しが行えるため、個々の呼び出しを区別できるようにする 必要があります。

ここでは、戻り値を使う方法と、レシーバの状態を分離する方法を説明します。

Option 1: 呼び出し状態を関数の戻り値に含める

計算中かどうかは関数呼び出しに紐づく状態であるため、戻り値で状態を表現すれば 1:1 の関係で表すことができます。典型的な方法としては、CompletableFutureDeferred のようなプロミスやフューチャーオブジェクトを返し、そこから計算中かどうかを取得すればよいでしょう。以下の SomeFuture では、isFinished を確認することで、個別に計算が実行中であるかを調べることができるようになっています。

class SomeFuture<T>(action: () -> T) {
    ...

    fun isFinished(): Boolean { ... }

    fun onFinished(subscriber: (T) -> Unit)
}

class FormulaCalculator {
    ...

    fun calculateSomeFormula(x: Int): SomeFuture<Int> =
        SomeFuture {
            ... // Takes long time to calculate
        }
}

Option 2: 関数呼び出しごとにレシーバの状態を分離する

仕様によっては、「何か実行中の計算があるか」を表示するために、個別の関数の状態ではなくレシーバの状態として管理したいこともあります。その場合は、単なる Boolean ではなく、個々の関数呼び出しを区別できる値を使うとよいでしょう。例えば、関数呼び出しごとにオブジェクトを作ってセットやマップで管理する方法や、実行中の呼び出しをカウントする方法が考えられます。今回の場合は、以下のように実行中の呼び出しをカウントするだけで十分です。

class FormulaCalculator {
    val isCalculating: Boolean get() = ongoingCalculationCount.get() > 0
    private val ongoingCalculationCount: AtomicInteger = AtomicInteger()
    
    fun calculateSomeFormula(x: Int): Int {
        ongoingCalculationCount.incrementAndGet()
        return try {
            ... // Takes long time to calculate
        } finally {
            ongoingCalculationCount.decrementAndGet()
        }
    }
}

「並行」でなくても又貸し

関数がシングルスレッドで実行される場合でも、再帰的に呼び出される可能性や割り込みが発生する可能性があるならば注意が必要です。以下のコードでは、calculateSomeFormula が再帰的に呼ばれているため、関数が完了していなくても isCalculating が false になってしまいます。

class FormulaCalculator {
    val isCalculating = false
        private set

    fun calculateSomeFormula(x: Int): Int {
        isCalculating = true
        if (x > 0)
            val z = calculateAnother(y)
            ...
        }
        isCalculating = false
    }

    private fun calculateAnother(y: Int): Int {
        ...
        calculateSomeFormula(value)
    }
}

関数を定義したときやレビューするときは、その関数の実行中に再度実行がされる可能性があるかを確認し、可能性があるときは正しく動作するかを検証する必要があります。

一言まとめ

関数呼び出しの状態を必要とするときは、個々の呼び出しを区別できるようにする。

キーワード: instance state, call state, re-entrancy

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

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