こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 54 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。(Weekly Report の詳細については、過去の記事一覧を参照してください)
沈黙は金メッキかも
以下のような API 呼び出しの結果を意味するデータモデルがあるとします。
このような「失敗するかもしれない」結果を意味するデータモデルに、map
や flatMap
といった関数を定義すると、操作をチェーンできるようになり、便利なことが多いです。
以下の例では、ApiResult
に mapSuccess
と flatMap
を追加しています。どちらも「成功時には次の操作を行う」というコードを、メソッドチェーンで書けるようにすることを意図しています。
このコードになにか問題はありますか?
良薬は口に苦し
このコードでは、 @file:Suppress("warnings")
によってファイル全体のすべての警告を抑制しています。ただし、実際に抑制したい警告は as ApiResult<S>
で発生する "unchecked cast" だけです。ファイル全体のすべての警告を抑制してしまうと、将来発生する警告も意図せず抑制してしまい、バグを見過ごしてしまう可能性があります。例えば、ライブラリの更新などで、使っている関数が Deprecated になったことを見落とす可能性もあるでしょう。
警告については、無視しない、過剰に抑制しない よう気をつける必要があります。警告が発生した場合は、以下のどちらかを実施する必要があります。(ただし、1 が優先されるべきです)
- 警告の原因を解決する
- 抑制の範囲を小さくとどめ、抑制の理由をコメントで説明する
1: 警告の原因を解決する
可能であれば警告を抑制せず、原因を根本から解決するのが望ましいです。今回の例では、タイプパラメータ T
を共変 (covariant) にし、ダウンキャストを削除することで警告を解決できます。Kotlin では、以下のように out
モディファイアを使うことで、T
を共変にできます。
仮に T
を共変にすべきでない (不変を維持するべき) という状況であるならば、Failed
のタイプパラメータを Nothing
から T
に変え、map
や flatMap
が呼ばれる度に Failed
インスタンスを作り直すという方法もあります。
2: 抑制の範囲を小さくとどめ、抑制の理由をコメントで説明する
警告の原因を解決できないときは、抑制せざるを得ないこともあります。警告の原因が外部のライブラリやフレームワークに起因する状況などが、その代表例です。他にも、解決案を思いつかなかったという状況もありえます。そのような場合は、以下の 3 点に気をつけてください。
- 抑制のコード範囲を限定する: ファイルレベルの抑制よりも、クラス、関数、ステートメントレベルの抑制を選ぶ
- 抑制の対象となる警告を限定する: "warnings" や "all" といったすべての警告を抑制するのではなく、"UNCHECKED_CAST" のように警告の種類を特定する
- コメントで理由を書く: なぜ抑制が必要なのか、原因の解決ができないのかをコメントで説明する
コードを書いているときに警告の原因を解決できなかったとしても、コメントで抑制の理由を明確にしておくことで、コードレビューアや他の開発者が解決案を思いつく可能性があります。ApiResult
の例では、以下のように "UNCHECKED_CAST" の理由を明記しておくことで、誰かが共変の利用やインスタンス再作成のような解決案を思いつけるかもしれません。
一言まとめ
警告はまず原因の解決を試み、抑制が必要なら範囲を限定した上でコメントを書く。
キーワード: warning
, suppression
, comment