どちらのコードが好ましい？5問目（Kotlin Fest 2024 Pocket Code Battle）

こんにちは。Androidアプリエンジニアの大石です。

先日開催されたKotlin Fest 2024のLINEヤフー企業ブースでは、「Pocket Code Battle」を実施しました。「Pocket Code Battle」とは、同じ動作をする複数の異なる実装方法のコードを比較し、どちらが好ましいかを考えてもらう企画です。

ブースに来ていただいた方には意見を書いた付箋を貼ってもらい、その意見を元にLINEヤフーの開発者と交流しました。

本記事では、今回出題した「Pocket Code Battle」の5問目の解説をします。

#5 Time will tell

5問目は長編問題となっており、仕様の変化に伴ってコードがどのように変わるかがテーマになっています。

問題の想定ですが、サーバーのAPIを呼び出す callFoo() という関数があるとします。これは失敗時に IOException をthrowします。

class FooRequest { ... }
class FooResponse { ... }
   
@Throws(IOException::class)
suspend fun callFoo(request: FooRequest): FooResponse { ... }
  
const val MAX_RETRY = 3

この関数を呼び出し、失敗したら最大MAX_RETRY回（この例では3回）までリトライをする callFooWithRetry() 関数を実装するならどのようになるでしょうか？

コード1-A

suspend fun callFooWithRetry(request: FooRequest): FooResponse =
    suspend { callFoo(request) }.asFlow()
        .retry(MAX_RETRY) { e -> e is IOException }
        .single()

コード1-B

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var attempt = 0
    while (true) {
        try {
            return callFoo(request)
        } catch (e: IOException) {
            if (attempt == MAX_RETRY) {
                throw e
            }
        }
        attempt++
    }
}

次に、この callFooWithRetry() 関数に「n回目のリトライを行う前にはn秒のディレイを入れる」という仕様を追加してみましょう。

コード2-A

suspend fun callFooWithRetry(request: FooRequest): FooResponse =
    suspend { callFoo(request) }.asFlow()
        .retryWhen { cause, attempt ->
            if (attempt < MAX_RETRY && cause is IOException) {
                delay(timeMillis = (attempt + 1) * 1000L)
                true
            } else {
                false
            }
        }
        .single()

コード2-B

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var attempt = 0
    while (true) {
        try {
            return callFoo(request)
        } catch (e: IOException) {
            if (attempt == MAX_RETRY) {
                throw e
            }
        }
        attempt++
        delay(timeMillis = attempt * 1000L)
    }
}

最後に、callFoo() や callFooWithRetry() 関数の返り値の型を Result<FooResponse> に変えて、失敗時には例外をthrowするのではなく Result.falure を返すように仕様変更してみます。

// 変更後のcallFoo()関数
suspend fun callFoo(request: FooRequest): Result<FooResponse> { ... }

コード3-A

suspend fun callFooWithRetry(request: FooRequest): Result<FooResponse> =
    suspend { callFoo(request).getOrThrow() }.asFlow()
        .retryWhen { cause, attempt ->
            if (attempt < MAX_RETRY && cause is IOException) {
                delay(timeMillis = (attempt + 1) * 1000L)
                true
            } else {
                false
            }
        }
        .map { Result.success(it) }
        .catch { e ->
            if (e !is IOException) {
                throw e
            }
            emit(Result.failure(e))
        }
        .single()

コード3-B

suspend fun callFooWithRetry(request: FooRequest): Result<FooResponse> {
    var attempt = 0
    while (true) {
        val result = callFoo(request)
        if (result.isSuccess || attempt == MAX_RETRY) {
            return result
        }
        attempt++
        delay(timeMillis = attempt * 1000L)
    }
}

これらのコードは、AのほうはコルーチンのFlowを用いており、Bは明示的なループ構文を用いているという違いがあります。

この問題には非常に多くのコメントをいただきました。以下にいくつか抜粋します。

1番目のコード
- 1-A: 短くてシンプルなので良いと思う。Kotlinらしいコードで好き。
- 1-A: Flowをこのように扱えることを初めて知った。
- 1-A: "retry"という名前の関数を使っていることでリトライの意図がわかりやすい。
- 1-B: var変数への再代入はできる限り避けたいので、Aのコードのほうが良い。
- 1-B: "while (true) { ... }"と書くと無限ループの危険がありそうで怖い。代わりに "repeat(MAX_RETRY) { ... }" を使うべき。

2番目のコード
- 2-A: 少しコードが複雑になったが、まだAのほうが良いと思う。
- 2-A: やはり無限ループを使わないAのほうが好き。
- 2-A: "retryWhen"の引数のラムダ式が大きいので、別関数に分けたほうが良い。
- 2-A: 一つのラムダ式内でリトライの判定とディレイの両方を行っているのがちょっとわかりにくい。
- 2-B: 1-Bに1行加えるだけで実装できることに驚いた。
- 2-B: 1はAのほうが良いと思うが、2の時点で自分ならBの方法に書き換えるだろう。

3番目のコード
- 3-A: さすがに複雑すぎる。
- 3-A: throwとcatchが入り混じっていてわかりにくい。
- 3-A: 冗長で可読性が低いコードではあるが、それでも無限ループを使っていないのでこちらのほうが安心する。
- 3-B: Aと比べると非常にわかりやすい。
- 3-B: Resultの場合だとBのほうが明らかにシンプルに書けることに驚いた。

解説

Flow.retry() 関数を使うと例外を投げる処理のリトライを簡潔に書くことができます。同様の retry() 関数は RxJava にもあるので、以前から RxJava を使っていた方々にはなじみがあるでしょう。

この Flow.retry() 関数を使うときは { e -> e is IOException } のように例外の型をチェックするのを忘れないようにしてください。そうしないとどんな例外に対してもリトライしてしまい、例えばバグによって RuntimeException がthrowされたときにもリトライしてしまいます。リトライは、それをしてもよい失敗のときにだけ行うべきです。

また、Flow.retry() 関数はリトライ回数に応じた処理を記述できません。コード2-Aのように、リトライ回数に応じたディレイを入れるような場合は Flow.retryWhen() を使う必要があります。

1-Bのコードについて、while (true) のところで無限ループを気にしている方が非常に多いことが印象的でした。いただいたコメントにもありますが、これは repeat を使って以下のように書くこともできます。

コード1-Bの別解

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var exception: IOException? = null
    repeat(MAX_RETRY + 1) {
        try {
            return callFoo(request)
        } catch (e: IOException) {
            exception = e
        }
    }
    throw exception!!
}

repeatの引数には MAX_RETRY + 1 を指定しなければならないことに注意してください。「リトライを最大でMAX_RETRY回行う」のですから、callFoo()関数は最大で MAX_RETRY + 1 回実行されることになるからです。また、最後の行においては exception が null になることはないはずですが、Kotlinコンパイラはそれを推論できないので exception!! のように書かなければなりません。

2-Bのコードも同様に repeat を使って書き換えることができます。ただし、以下のように書くのは正しくありません。

コード2-Bの別解（間違い）

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var exception: IOException? = null
    repeat(MAX_RETRY + 1) { attempt ->
        try {
            return callFoo(request)
        } catch (e: IOException) {
            exception = e
        }
        delay(timeMillis = (attempt + 1) * 1000L)
    }
    throw exception!!
}

このコードでは、最後のリトライで失敗した場合にも exception をthrowする前にディレイが入ってしまいます。

正しくは、以下のようにループの先頭に delay 文を入れる必要があります。

コード2-Bの別解

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var exception: IOException? = null
    repeat(MAX_RETRY + 1) { attempt ->
        delay(timeMillis = attempt * 1000L)
        try {
            return callFoo(request)
        } catch (e: IOException) {
            exception = e
        }
   }
    throw exception!!
}

なお、このコードは最初の実行の際（attempt == 0のとき）に delay の引数が 0 になることに依存しています。ディレイ間隔のポリシーを異なるものにした場合（例えばリトライ回数によらず常に2秒のディレイを入れるなど）は、ループの最初にディレイが入らないように注意しましょう。

コード2-Bの別解（ディレイ時間固定）

suspend fun callFooWithRetry(request: FooRequest): FooResponse {
    var exception: IOException? = null
    repeat(MAX_RETRY + 1) { attempt ->
        if (attempt != 0) {
            delay(timeMillis = 2000L)
        }
        try {
            return callFoo(request)
        } catch (e: IOException) {
            exception = e
        }
   }
    throw exception!!
}

このように、while (true) の代わりに repeat を使った場合でも、バグになりそうな注意点は意外と多いです。コード3-Bも同様に repeat を使って書き換えることができますが、これは読者の皆さんへの練習問題としましょう。

最後の Result<FooResponse> を返すバージョンについては、圧倒的にBのほうが人気でした。Flowの retry() あるいは retryWhen() は例外がthrowされたときだけリトライできます。そのため、Result<T>の結果に応じてリトライをしたいときに用いるのは適切ではないでしょう。

おわりに

「Pocket Code Battle」の5問目について解説しました。Flowのオペレータには便利なものがそろっていますが、融通が利かないことも多いので利用には注意が必要です。そして、明示的なループを用いたコードは「あまりモダンではない」といった評価を受けることもありますが、むしろ直感的でわかりやすいコードになることも多いです。

6問目以降の解説記事についても後日掲載いたします。お楽しみに！