Skip to main contentSkip to footer
Search
LINEヤフー Tech Blog

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

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

こんにちは。AndroidアプリエンジニアのChigitaとFukunoです。

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

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

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

#1 Haste makes waste

1問目には以下のコードを出題しました。

コード#1-A

data class Top2Result(
    val largest: Long,
    val secondLargest: Long
)

fun findTop2Value(longSet: Set<Long>): Top2Result {
    val sortedList = longSet.sortedDescending()
    return Top2Result(sortedList[0], sortedList[1])
}

コード#1-B

fun findTop2Value(longSet: Set<Long>): Pair<Long, Long> {
    var currentLargest = Long.MIN_VALUE
    var currentSecondLargest = Long.MIN_VALUE
    for (value in longSet) {
        if (value > currentLargest) {
            currentSecondLargest = currentLargest
            currentLargest = value
        } else if (value > currentSecondLargest) {
            currentSecondLargest = value
        }
    }
    return currentLargest to currentSecondLargest
}

これは、引数を通して与えられた Set<long>  の中から最も大きい値と、2番目に大きい値を見つけ返却する関数についての問題です。コード#1-Aでは、 longSet を降順ソートし、ソート列から0番目と1番目を指定する方法です。コード#1-Bは、 longSet  を for 文で走査し、最も大きい値と、2番目に大きい値を currentLargest  と currentSecondLargest  に保持しながら見つける方法です。

コメント

実装内容がシンプルなため、Pocket Code Battle のチュートリアル問題として多くの人に挑戦していただき、たくさんのコメントをいただきました。会場でのコメントの傾向としてはAのほうが好ましいが改善できるという声が多くありました。

コード#1-Aについて

  • ソート関数を自分で実装せず、Standard Libraryを使っているためバグが発生しにくい
  • データクラスで戻り値をまとめていてcallerが使用を間違えにくい
  • Top3が必要になっても、変更のコストが小さい
  • longSet のsizeが2未満だと例外が発生してしまう

コード#1-Bについて

  • 計算量の観点で考えると、コード#1-Aは O(NlogN)  だが、コード#1-Bは O(N)  なので後者のほうが処理が早い
  • 例外が発生しない
  • 関数の実装を読まないと、戻り値の Pair  の first  と second にそれぞれ何が入っているのかがわからない

解説

この問題の大きなポイントは3つです。

  1. 計算量の観点
  2. 変更容易性の観点
  3. 例外処理の観点

1. 計算量の観点

計算量はコメントでも指摘があった通り、コード#1-Aはソートを行うため O(NlogN)、コード#1-Bは set  の各要素を一度のみ確認するため O(N) です。そのため、実行の速度は longSet  のサイズに依存することがわかります。引数として渡される longSet  のサイズが常に大きく、この処理がパフォーマンスクリティカルな場合には、コード#1-Bを選択することも可能でしょう。そのような場合には適切・適当な理由をメソッドコメントとして残すべきです。

2. 変更容易性の観点

思考実験として、後の仕様変更として 1. 最大値3つが必要になったり、2. 最大値2つと最小値が必要になったと仮定しましょう。このときに、より少ない変更量で修正できるのはどちらのコードでしょうか？　よりバグの発生可能性を少なく修正できるのはどちらのコードでしょうか？　コメントでもあったように多くの方は、コード#1-Aを選択するでしょう。関数のコアのロジックがStandard Libraryで実装されている、戻り値が data class  でまとめられているため、caller / callee双方でバグの入り込む余地が少ないと言えます。つまり、コード#1-Aのほうが変更容易性が高いと言えます。このような思考実験はアドホックに変更容易性を考慮するのに便利です。しかし注意点として、思考実験でむやみに存在しない条件を追求しすぎると、YAGNIに反する実態と乖離（かいり）した関数ができるため、やり過ぎには注意しましょう。

3. 例外処理の観点

コメントで指摘があったように、コード#1-Aは longSetのサイズが2未満のときに Index Out Of Bounds Exception がthrowされます。一方コード#1-Bでは例外はthrowされないものの、 Long.MIN_VALUE  が結果として返却されます。ケースバイケースではありますが、多くの場合これはどちらも望ましくありません。コード#1-Aは、今回のケースではTop2が取得できなかったことは null を返却することでcallerに知らせることができます。そのためには、サイズをあらかじめ検証してearly returnすることで解決できます。コード#1-Bも同様に null  を返却することが望ましいです。例えば、longSet が setOf(Long.MIN_VALUE, Long.MIN_VALUE) だったときに、caller側は longSet のサイズが2未満だった場合のエラー（デフォルト値へのフォールバック）と区別ができません。区別が不要な場合はその限りではありませんが、区別されるのが望ましいと考えています。より詳しい解説はこちらを御覧ください。
https://techblog.lycorp.co.jp/ja/20231109a

#2 The platform implementation is your oyster

2問目には以下のコードを出題しました。

コード#2-A

// In the commonMain
expect class Environment {
    val platformSpecificValue: String
}

// In the androidMain
actual class Environment {
    actual val platformSpecificValue: String = "Android"
}

// In the iosMain
actual class Environment {
    actual val platformSpecificValue: String = "iOS"
}

コード#2-B

// In the commonMain
interface Environment {
    val platformSpecificValue: String
}
expect val environment: Environment

// In the androidMain
actual val environment: Environment = AndroidEnvironment()
class AndroidEnvironment : Environment {
    override val platformSpecificValue: String = "Android"
}

// In the iosMain
actual val environment: Environment = IosEnvironment()
class IosEnvironment : Environment {
    override val platformSpecificValue: String = "iOS"
}

KMPで platformSpecificValueをプラットフォームごとに書き換える処理を実装する際のexpect / actual キーワードに関する問題です。コード#2-Aでは、classに直接 expect / actual キーワードを使用しています。一方で、コード#2-Bでは interface に一度切り出した上で val に対して expect / actual キーワードを使用しています。

コメント

A、Bほとんど同じ量のコメントがつけられましたが、Bのほうが良いという声がわずかに多かったです。

コード#2-Aについて

  • interface は課題に対して過剰な手法で、Aのほうがシンプルである
  • expect classは Beta である。

コード#2-Bについて

  • interface に切り出すことでテスト時のモックが扱いやすい
  • プラットフォーム内の実装の拡張性が高い

解説

コード#2-Aはシンプルに実現したいことを実装しているため、デモ実装などではこちらで十分でしょう。しかし以下の2点において懸念があります。

  1. classに対して直接 expect / actual キーワードを指定すると、各プラットフォームでactual classとしての実装を強制してしまう。
  2. expect class は stability level が Beta なので、breaking changeが将来起きる可能性がある。

特に、1についての懸念が大きいと個人的には考えています。interfaceにすることで、testなどで実装を用意に差し替えることができたり、各プラットフォームの関心事に合わせて実装を柔軟に選択することできます。詳細については、こちらでも解説されていますので御覧ください。
https://kotlinlang.org/docs/multiplatform-expect-actual.html#expected-and-actual-classes（外部サイト）

2 については、KotlinではStability levelを Experimental、Alpha、 Beta そして Stable の4段階で表現しています。Experimental や Alpha の状態でプロダクションで採用することは負債となる懸念や、移行コストを考える必要がありますが、Beta以降であれば、影響は小さく抑えられるのではないでしょうか？　ただしこれはプロダクトの性質に強く依存する部分になっています。こちらも詳細は Kotlinのドキュメントを参照してください。
https://kotlinlang.org/docs/components-stability.html#stability-levels-explained（外部サイト）

#3 All that glitters is not immutable

3問目には以下のコードを出題しました。

コード#3-A

sealed interface OutputResult {
    class Success(val sum: Int, val max: Int) : OutputResult
    class Failure(val e: Exception) : OutputResult
}
sealed interface InputResult {
    class Success(val value: Int) : InputResult
    class Failure(val e: Exception) : InputResult
}
 
fun calculate(inputList: List<InputResult>): OutputResult {
    var sum = 0
    var max = Int.MIN_VALUE
    for (item in inputList) {
        when(item) {
            is InputResult.Failure ->
                return OutputResult.Failure(item.e)
            is InputResult.Success -> {
                sum += item.value
                max = item.value.coerceAtLeast(max)
            }
        }
    }
    return OutputResult.Success(sum, max)
}

コード#3-B

sealed interface OutputResult {
    class Success(val sum: Int, val max: Int) : OutputResult
    class Failure(val e: Exception) : OutputResult
}
sealed interface InputResult {
    class Success(val value: Int) : InputResult
    class Failure(val e: Exception) : InputResult
}

fun calculate(inputList: List<InputResult>): OutputResult =
    inputList.fold(
        OutputResult.Success(0, Int.MIN_VALUE)
    ) { acc: OutputResult, result ->
        when (acc) {
            is OutputResult.Failure -> return acc
            is OutputResult.Success -> when (result) {
                is InputResult.Failure ->
                    OutputResult.Failure(result.e)
                is InputResult.Success -> {
                    val sum = acc.sum + result.value
                    val max = result.value
                        .coerceAtLeast(acc.max)
                    OutputResult.Success(sum, max)
                }
            }
        }
    }

与えられた inputResult のリストから総和と最大値を計算し、 OutputResult  として返却するコードに関連する問題です。コード#3-A では、 sum  と max  を var として保持して計算します。コード#3-B では fold を用いて計算します。

コメント

意見は大きく3つに分かれました。Aがシンプルで良い、Bはvarではなくvalを使っているので良い、どちらも読みづらいの3つです。意見が大きく割れたため、出題者としてはこのように多様な意見を聞くことができる問題を作成できて嬉しく思います。

コード#3-Aについて

  • var は可能な限り使いたくないが、可読性の観点で優れている
  • ネストが少なくて読みやすい
  • forの中の深い箇所でreturnするため、構造を追いにくい

コード#3-Bについて

  • immutableを意識している点で優れている。変数はvalであるべき。
  • foldを使ってうまく書けている

解説

この問題には2つの観点があります。

1. varの使用について

varによって変数を宣言し、再代入を繰り返すことは広くアンチパターンとして知られています。広範囲なスコープから参照できる再代入可能な変数があると、その変数が意図しない箇所で変更される可能性があり、コードを読む際の負荷が大きくなります。一方で、今回のケースのように限られたスコープであれば、varを用いることを考慮してもよいでしょう。私個人の考えとしては、まずimmutableな方法でコードを書き、煩雑でリーダビリティの低いコードしか書けない場合は、forループとvarを用いて書き直すという手順を踏むと良いと考えています。

2. Resultの加工について

問題では、InputResultのリストからOutputResultに変換する操作が必要です。このような操作はKotlinでは煩雑になる傾向にあります。一つのアイデアとして、Failureが含まれている場合にはearly returnを行う方法があります。この操作を行う際に、successとFailureのリストに分割することで、whenのネストの深さが解消され、リーダビリティも向上します。Kotlinのsealed interfaceは網羅性を保証できる一方で、今回の問題のように処理のネストが深くなることがあります。適切にearly returnを行ったり、フィルタしたリストを作成することで、網羅性とリーダビリティの双方を維持できます。

#4 Good things come in small packages

4問目には以下のコードを出題しました。

interface CompanyDataSource {
    /**
     * Fetch the Company associated with a given [id]. E.g., 4689 for LY Corporation.
     * @throws NoSuchElementException If no company corresponds to the given id.
     * @throws IOException If an I/O error occurs.
     */
    fun get(id: String): CompanyResponse
}

証券コードを引数に取り、会社情報を返すget関数があるとします。この関数は、コール失敗時に2種類の例外をスローします。

  • NoSuchElementException 引数に対応するエントリが見つからない場合
  • IOException I/Oエラーの発生

この関数へのアクセスを抽象化するfetchCompany関数を別クラスに作りたいとき、以下のどれが良いと思いますか？

コード#4-A

fun fetchCompanyA(companyId: String): Company =
    dataSource.get(id = companyId).let {　… }

コード#4-B

fun fetchCompanyB(companyId: String): Result<Company> =
    runCatching { dataSource.get(id = companyId).let { … } }

コード#4-C

fun fetchCompanyC(companyId: String): Company? =
    runCatching { dataSource.get(id = companyId).let { … } }
        .getOrNull()

コード#4-D

fun fetchCompanyD(companyId: String): CompanyResult =
    runCatching { 
        dataSource.get(id = companyId)
            .let { … }
            .let { CompanyResult.Success(it) }
    }.getOrElse { throwable ->
        when (throwable) {
            is NoSuchElementException ->　CompanyResult.NotFoundError
            else -> CompanyResult.UnknownError
        }
    }

sealed interface CompanyResult {
    data class Success(company: Company): CompanyResult
    data object NotFoundError: CompanyResult
    data object UnknownError: CompanyResult
}

リポジトリパターンでデータソースへのアクセスを抽象化することを想定した出題です。抽象化対象のget関数が2種類の例外をスローするので、それをどのように扱うかがポイントとなります。

  • コード#4-Aはget関数で発生した例外をそのまま受け流す作戦
  • コード#4-BはResult型にする作戦
  • コード#4-Cは例外発生の場合nullにする作戦
  • コード#4-Dはsealed interfaceを使う作戦

コメント

Kotlin Fest当日、来場者の方からは特にDの実装パターンを支持する声が多く得られました。

コード#4-Dに対する意見

  • 失敗したとき、呼び出し側にとってその理由がわかりやすい
  • Kotlinらしくて好き
  • sealed interfaceはいいぞ

実装上わかりやすいという声がありましたが、それ以上にsealed interfaceという仕組みを持つKotlinへの愛を感じる意見も多く見られました。見ていて笑顔になりますね。私も好きです。

他の選択肢に対する意見

若干数だけ、他の実装パターンを支持する意見も得られました。

  • A,B,Cは既存コードの制約が大きい場合に選択できる
  • A,Bは例外・スタックトレースの情報を消さずに呼び出し側に伝えられる
  • Cはエラー時の処理を分ける必要がない場合に選択できる

後述しますが場合によってどの実装が適切かは変わるので、こちらも重要な観点だと思います。

解説

どの実装パターンが適切か判断するためには、以下の2つの観点が重要です。

  • 例外ハンドリングが必須かどうか
  • 失敗の種類に区別をつける必要があるかどうか

コード#4-A（get関数で発生した例外をそのまま受け流す作戦）

  • [不可能] 例外ハンドリング
  • [部分的に可能] 失敗の区別

Aの実装方法を採った場合、NoSuchElementExceptionやIOExceptionが発生した場合に利用側（アクセスを抽象化しているレイヤの外）でtry-catchが必要です。

しかしKotlinには検査例外の仕組みがなく、言語設計としてtry-catchを利用側で書くような方向性ではありません。日常的に例外が発生し、例外をハンドリングする必要があるのなら、適さない選択だと思います。

一方で例外ハンドリングが不要ならこれが一番シンプルです。

コード#4-B（Result型にする作戦）

  • [可能] 例外ハンドリング
  • [部分的に可能] 失敗の区別

Bの実装方法を採った場合、KotlinのResult型を活用することで失敗をResult.Failureとして扱えます。成功した場合はResult.Successです。利用側はResult.getOrElse()とかResult.fold()とかを使って、成功の場合と失敗の場合それぞれのハンドリング処理を書くことができます。

try-catchと違って確実に例外ハンドリングさせられるところがポイントです。

かなり良さそうですが、問題点もあります。失敗したときに、どのような理由で失敗したかの区別をつけるにはNoSuchElementExceptionやIOExceptionといった具体的な例外クラスを元に判定しなければいけません。リポジトリパターンでデータソースへのアクセスを抽象化するという今回の想定から言ったら、リポジトリの関心事が利用側に漏れ出しているような気配があります。

またrunCatchingでResult型を作る手法はあらゆる例外をResult.Failureにして握りつぶしてしまうので、正確な例外ハンドリングを行うことにはあまり向かないような書き方になっています。

コード#4-C（例外発生の場合nullにする作戦）

  • [可能] 例外ハンドリング
  • [不可能] 失敗の区別

Cの実装方法を採った場合、失敗をnullとして扱います。KotlinにはNull safetyの仕組みがあるので、利用側はElvis operatorなどを使って失敗の場合の処理を書くことができます。Bと同様、try-catchと違って確実に例外ハンドリングさせられるところがポイントです。

Kotlinの標準APIにもString.toIntOrNull()のように失敗を全てnullにするものがあるので、それに近い実装方法と言えるでしょう。ただし「失敗なら全てnull」なので、どのような理由で失敗したかの区別は完全につきません。

区別が必要なら適さない選択ですが、区別がいらないならこれが一番シンプルです。

コード#4-D（sealed interfaceを使う作戦）

  • [可能] 例外ハンドリング
  • [可能] 失敗の区別

Dの実装方法を採った場合、失敗はその種類ごとにCompanyResult.NotFoundErrorやCompanyResult.UnknownErrorとして扱えます。成功した場合はCompanyResult.Successです。

利用側はwhen文を使って各種の失敗や成功それぞれのハンドリング処理を書くことができます。失敗の種類別の例外ハンドリングをさせられるところがポイントです。

最も安心確実な実装方法と言えます。他の実装パターンと比べて非常に長いですが。

どれを使うべきか？

今回の出題の場合、「証券コードを引数に取り、会社情報を返す」という想定から言ってなんとなく以下が推測できます。

  • 失敗時の例外ハンドリングが必要
  • 不正な証券コードが渡ってきて失敗したときと、I/Oエラーなど他の理由で失敗したときの区別が必要

この場合、D>B>C>Aの順で適切と言えるでしょう。会場でDが人気だったのは出題者の想定通りでした。

おわりに

「Pocket Code Battle」の1問目-4問目について解説しました。

この記事で何か新しい発見はありましたか？　今回のような取り組みを通してKotlin界隈をもっと盛り上げられたら嬉しいです。

また後日、5問目以降の出題についても解説記事を投稿予定です。お楽しみに！