Zenn
📝

Kotlinでの!!とrequireNotNullとcheckNotNullとerrorの使い分け

2020/09/27に公開
Kotlin
tech

https://twitter.com/kotlin/status/1309153139753390083 は明らかに正しいと思うので、自分の好みをメモしておく。

TL;DR

  • 状態、引数のチェックに応じて require, check(error) は使い分けられると良さそう。
  • a clear message は文脈に応じて変わるが、表現出来ると便利そう。
  • 正直使い分けや a clear messageの構築は面倒くさそうだが、それでも !! が適切そうなケースはほぼなさそう。

(と思っている)

はじめに

a clear message とは

残念ながら「文脈に応じて変わる」としか言いようがないと思います。後述する checkNotNull のデフォルトメッセージは "Required value was null." ですが、それがその文脈で 的確に状況を説明している と言えるのであれば、それもまた clear と呼ばれるべきでしょう。
他にも例えば状態の説明・表現は clear と言えるでしょうし、利用しているクラッシュレポートツールやメトリクス解析ツールなどの外的因子によっては構造化テキストが期待されたり、一定の文法に従うこと自体が clear とも言えるでしょう。それはチームやプロダクトによって大きく異なります。具体的に「何が a clear message なのか」については触れることは難しいですが、誰か個人が「自分はそのメッセージを clear だとは思わない」と感じたとしても「Prefer to throwing an error with a clear message を否定することにはならない」と思います。
仮に error("foo was null") (あるいはデフォルトメッセージの checkNotNull) ばかり溢れてしまうとしたら、それは「前提条件が分かりやすい設計のため、本当にそれで十分」か「個人またはチームにおけるコード品質に問題がある」と考えられると思っています。したがって、「メッセージを考える難易度」と「!! や a clear meesage を使い分けること」は全くの別物だと思っており、また少なくとも元ツイートでは前者の内容については一切触れていないと思っています。

要するに、「a clear message と呼べるエラーメッセージや説明があるとしたら」という前提に立って、!! と throwing an error with a clear message を比較検討するべきだと考えています。

それぞれの Signatures

fun <T : Any> requireNotNull(value: T?): T
fun <T : Any> requireNotNull(value: T?, lazyMessage: () -> Any): T
fun <T : Any> checkNotNull(value: T?): T
fun <T : Any> checkNotNull(value: T?, lazyMessage: () -> Any): T
fun error(message: Any): Nothing

それぞれで出来ることと投げられる例外について

var foo: Int? = null
書き方 例外クラス メッセージ 備考
foo!! java.lang.NullPointerException null
requireNotNull(foo) java.lang.IllegalArgumentException "Required value was null."
requireNotNull(foo) { "hello" } java.lang.IllegalArgumentException "hello" lambda の返り値.toString() が Throwable#message にセットされる
checkNotNull(foo) java.lang.IllegalStateException "Required value was null."
checkNotNull(foo) { "hello" } java.lang.IllegalStateException "hello" lambda の返り値.toString() が Throwable#message にセットされる
foo ?: error("hello") java.lang.IllegalStateException "hello" error の引数(Any).toString() が Throwable#message になる
foo ?: throw XException("hello") XException "hello" 任意の例外を投げられる

return を利用したguard文相当の動きについては !! を除いてどれでも実現が可能なため、差異は特にありません。

それぞれの例外の意味

以下はJavadoc に書いてあるものを転記したもので、自分自身は基本的にこの基準をベースにしているというだけです。「自分たちはチームとして違う使い方を受容しているぞ」と言うなら話は変わるかもしれません。

java.lang.NullPointerException

Thrown when an application attempts to use null in a case where an object is required. These include:

  • Calling the instance method of a null object. -> Kotlin だと無理にやらないとないですね
  • Accessing or modifying the field of a null object. -> Kotlin だと(ry
  • Taking the length of null as if it were an array. -> Kotlin だと(ry
  • Accessing or modifying the slots of null as if it were an array. -> Kotlin でもありそう
  • Throwing null as if it were a Throwable value. -> Kotlin でもありそう

例えば以下のようなAPIの場合も NPE で良さそうですね。(NPE見ると蕁麻疹が出る人を除きます)

fun getItem(position: Int): T {
  return array[position] ?: throw NullPointerException("Value at $position of Array was null") // or !!
}

java.lang.IllegalArgumentException

Thrown to indicate that a method has been passed an illegal or inappropriate argument.

引数が不正である、あるいは適切でないとするときに投げるとあります。まあ引数関連なことは間違いないですね。

java.lang.IllegalStateException

Signals that a method has been invoked at an illegal or inappropriate time. In other words, the Java environment or Java application is not in an appropriate state for the requested operation.

不正または適切じゃないタイミングにそのメソッドが呼ばれた、とあります。「何が」不正/適切じゃないという点に関しては、IllegalArgumentExceptionと台頭させるなら「引数以外」と見ると納まりが良さそうですね。

使い方アレコレ

個人の好みです(大事なことなので何度でもいいます)

class Bar {

  // これは nullable で受ける必要がないのでそもそも考えなくて良いはず
  fun doSomething1(foo: Foo?) {
    foo!!
    requireNotNull(foo)
    checkNotNull(foo?.bar())
    ...
  }
  
  var stateBasedInt: Int?
  var nonNullIfInitializeWasCalled: Int?

  fun doSomething(foo: Foo) {
    // Bar の状態が期待と異なるので良さそう
    checkNotNull(stateBasedInt) // IllegalStateException
    
    // 引数のチェックじゃないので違いそう
    requireNotNull(stateBasedInt) // IllegalArgumentException
    
    // 「必要条件」or「当然そうなるはず」かが一切分からないのでダメそう
    stateBasedInt!! // NPE
    
    // Bar の状態が期待と異なるので良さそうに見えるけれど、そもそも「初期化しなければいけないのにされてない」とかならlateinitにした方が良い可能性もある(設計次第)
    // requireNotNull は引数じゃないので違いそう
    checkNotNull(nonNullIfInitializeWasCalled) // IllegalStateException
    
    // 引数自体の状態のチェックとも言えるが、引数に関しては Argument が正しそう 
    checkNotNull(foo.nonNullValueIfFooIsInitialized) // IllegalStateException
    
    // 期待している引数と異なるので良さそう
    requireNotNull(foo.nonNullValueIfFooIsInitialized) // IllegalArgumentException
    
    // やはり「必要条件」or「当然そうなるはず」かが一切分からないのでダメそう
    foo.nonNullValueIfFooIsInitialized!! // NPE
    
    // 引数ではなく、Quxの状態に関するものなので良さそう
    checkNotNull(Qux.mayBeNonNull) // IllegalStateException
    
    // 引数ではないので違いそう
    requireNotNull(Qux.mayBeNonNull) // IllegalArgumentException

    // smart castが効かない場合に良さそう(shadowingは考えないものとする)
    if (foo.nonNullValueIfFooIsInitialized != null) {
        val nonNullValue = requireNotNull(foo.nonNullValueIfFooIsInitialized) // IllegalArgumentException
    }
    
    // 同上
    if (stateBasedInt != null) {
        val stateBasedInt1 = checkNotNull(stateBasedInt) // IllegalStateException
        val stateBasedInt2 = stateBasedInt ?: error("...") // IllegalStateException
        val stateBasedInt3 = stateBasedInt!!
    }
   
    // 期待している点がインスタンスの状態ではなく、ライブラリ実装(状態)であることをエラーメッセージで表現しているので良さそう
    val recyclerView = 3rdPartyView.getChildAt(0) as? RecyclerView ?: error("3rd party View library の実装が変わった") // IllegalStateException
    // キャストの失敗が null 表現(NPE)に落ちていてNPEの利用方法としてもよくなさそうだし、理由説明が欠落している。
    val recyclerView = (3rdPartyView.getChildAt(0) as? RecyclerView)!! // NPE
    // なぜ(実装依存)、が欠けているので、NPEよりマシだが比較的弱そう
    val recyclerView = checkNotNull(3rdPartyView.getChildAt(0) as? RecyclerView) // IllegalStateException
    // (中略)良さそう
    val recyclerView = checkNotNull(3rdPartyView.getChildAt(0) as? RecyclerView) { "3rd party View library の実装が変わった" }
    
  }
  
  // これは null も計算結果として正常な場合として何かしら返すメソッド
  // (nullが常に異常ならこのメソッドがnon nullを返すべきで、そのチェックは呼び出し側の責務ではない)
  fun compute(arg1: Arg1, arg2: Arg2, ...): Quux? {
    ...
    return ...
  }
}

// 引数じゃないので違いそう
val computed1 = requireNotNull(Bar(...).compute(arg1 = arg1, arg2 = arg2, ...)) // IllegalArgumentException
// 呼び出し側の状態なのか、Bar 側の状態なのか不透明。ただ間違ってるとは言えなさそう。
val computed2 = checkNotNull(Bar(...).compute(arg1 = arg1, arg2 = arg2, ...)) // IllegalStateException
val computed3 = Bar(...).compute(arg1 = arg1, arg2 = arg2, ...) ?: error("Expected computed value was null") // IllegalStateException

おまけ - error と checkNotNull の使い分け

投げられる例外は一緒なので、ここも好みだと思います。なので自分の好みを書いておきます。

まず、error と checkNotNull どっちでも良さそうなら error の方が好きです。

error の利点は評価順そのままに記述することができるので、以下を(少なくとも僕にとっては)自然な順序で読む事ができます。

val foo = bar() ?: error("baz")

また checkNotNull は assertive に記述出来るため、テストコードでは Assertions とほぼ同等の感覚で好んで使います。

val foo = checkNotNull(bar()) { "baz" }

ここで大きく2種類の状況下では error を使わないようにしています。

  • Elvis 演算子を使いつつ、代入しない場合
  • Elvis 演算子が error の利用以前に現れる場合
/* Elvis 演算子を使いつつ、代入しない場合 */

// Elvis 演算子をまるで if 文のように使いつつ、そして smartcast 全振りという感じがします。
// IDE から開かない場合のコードリーディングだと、インタープリタの気分にならないといけないので好きではありません。
fun do...() {
  ...
  bar() ?: error("baz")
  ... // このあとに処理が続く
}

// 以下の方が好き
fun do...() {
  ...
  checkNotNull(bar()) { "..." } // assertive
  // or
  if (bar() == null) { // declative
    error("...")
  }
  ... // このあとに処理が続く
}

/* Elvis 演算子が error の利用以前に現れる場合 */

// 以下はnested 三項演算子みたいな印象で、あまり好きじゃない
(bar() ?: DEFAULT_VALUE).qux() ?: error("baz")

// こっちの方が好み。
// ただ正直こっちでも一回代入してくれないかな・・・という気持ちはある(それはまた別のお話)
checkNotNull((bar() ?: DEFAULT_VALUE).qux()) { "baz")

でももっと好みを言うと、ドメイン内の特定の状態違反に関しては自分たちで例外作ろうねって方針です。カスタム例外を作らない場合、例えば全部 Illegal State だと言われてもクラッシュレポートツールで見づらいですし、元ツイでいう a clear message を頑張ろうにも個々人の語学力やコードの出現箇所次第でブレブレになってしまうので・・・

おまけ - 元ツイの捉え方と立場

どう読んでも、error こそが良いという話ではない

元の英文を読めば分かる通り、!! よりも throwing an error with a clear message 方がオススメであるとしか言ってません。したがってerror/requireNotNull/checkNotNull/throw XException() といった例外を投げる手法の中で、errorをオススメするよ！という話では決して無いでしょう。

オススメしているだけで、優劣をつけているわけではない

原文を見ても優劣に言及はしていません。

自分の立場

throwing an error with a clear message に前のめりで賛成です。NPE を見ると蕁麻疹が出ます・・・というのは嘘ですが、NPEだろうがなんだろうが「なぜ」が欠けたコードは読みづらくてしょうがないですし、コードレビューのときに質問しなければならないときなどはコストになり得ると考えています。あとクラッシュしたときに行番号が出ても、その特定リビジョンのコードを見に行かないと分からないのは面倒くさいと感じます。原因箇所の書き手がクラッシュの読み手に状況理解の作業を押しつけてるだけでは? とまではさすがに思いませんが、その書き手でない場合はいちいちコードを見ないと意味が分からない・・・となるのは本当に面倒くさいです(2回目)。したがって、コメントによる説明よりもリッチなエラーメッセージによる説明の方が好みです。

まあこれは自分個人の経験として数年熟成されたコードベースのプロジェクトに後から入ることが多く、雑エラーメッセージとクラッシュに苦しめられた経験から、言語を問わず「説明のない例外処理とコードを見に行かないと分からないエラーレポートが嫌い」という私情が明らかに影響してそうです。
将来の自分やまだ見ぬ同僚に利するコード、そして自分がいない未来でもちゃんと役立つコードを書いていきたいですね。

Discussion