nullチェックなどのif文の記述場所について

Question

初歩的な質問で恐縮ですが・・・

あるitemがnullで無い場合にだけ処理を行う場合など
ある処理を発動する為の前提条件がある時に

private void Main()
{
    if (item != null)
    {
        // 何らかの処理
    }
}

こんな感じの処理を行うと思います

ですが、何らかの処理部分が長くなってしまい下記の様に別メソッドにした場合

private void Main()
{
    if (item != null)
    {
        SomeProcess()
    }
}


private void SomeProcess()
{
    // 何らかの処理
}

こんな感じになると思いますが
この場合の『if (item != null)』は
メソッドを呼び出す側(Main) or 呼び出される側(SomeProcess) どちらに記述するのが
一般的ですか？

独学で明確な基準を持たずにコードを書いている為
行き当たりばったりで書いてしまい混在してしまっているのが現状です

今後の指針にしたいので皆さんの書き方を参考にさせて頂けたら幸いです

Answer 1

暴力的な言い方をすると、呼び出す側がチェックをするのは例外を出さないため、
呼ばれる側がチェックするのは例外を投げるためです。
大きなプログラムやDLLを作るなら呼ばれる側は必ずチェックを入れますが、自分しか使わないような小さなものならどっちでもいいと思います。

Answer 2

メソッドを呼び出す側(Main) or 呼び出される側(SomeProcess) どちらに記述するのが一般的ですか？

状況によります。「両者とも外部から直接呼べる」場合など、両方に書くという選択肢もありえます。

Answer 3

バリデーションの責務というと最近こんな記事を見かけました

• Clean Architectureにおいてバリデーションはどこでやるべきか

結論としては「レイヤーごとの責務においてやるべき」というややふわっとした回答になります。

---

C# 8.0 以降であれば「null 許容参照型」を有効にすることで、参照型であっても「null/非null」を明示的にコードで示さなければならなくなるので、nullチェックの責務を負った地点以降のコードはnullチェックが不要になります。

• null 許容参照型 - Microsoft Doc

つまり、メソッドが非null参照型で受け取ったなら、それはnullチェック済みであることが自明となっているということであり、「null 許容参照型」を有効にすることはnullチェックの責務を負う場所の明確化を型システムレベルで求めることが出来るようになると言えます。

「null 許容参照型」を活用する前提で、改めてnullチェックの責務はどこで追うべきかを考えると、例えば入力を受け取る場所（UIとDomainを繋ぐプレゼンター層、またはDatabaseからデータ取得するリポジトリ層など）においてチェックし、いわゆるモデル層、ビジネスロジック部分で受け取る時は常に非nullで受け取る（ただし内部的なプロパティがオプション扱いである場合にnull許容として明示的にマークする）といったことが考えられると思います。

Answer 4

このケースについてのみ言うならば，nullチェックをどっちでやろうがどうでもいい．

SomeProcess とは，Main内の処理記述の一部を「長ったらしいから」という理由だけで切り出しただけの存在なので，
要は，その切り出す範囲を

• 処理だけ
• if( nullじゃないなら )処理

のどっちとするか，というだけの話だ．

---

で，もうちょっと真面目な(?)ことを考えると，
メソッドを作る(作った)時点で，そのメソッドの仕様をちゃんと固めればいい．
「このメソッドは○○がnullの場合にはこのようになります」という話が定まっていればよいだけ．

「このようになります」ということが受け入れられないならば，呼び出し側でチェックして呼ばないようにするだろうし，
「このように」なっても別にいいならば，呼び出し側はnullを気にせずに呼んでもいい．

※呼ばれる側でnullチェックするか？　には疑問の余地はない．「このように」するのにチェックが必要ならばするし，いらないならしなくてもよい．

Answer 5

安全を期すなら、呼ばれる側 SomeProcess() でチェック、でしょう。
但し、複数の関数を呼び出していて、それぞれの関数の中でチェックするのは無駄だから、呼び出す側で
一回だけチェックしよう、も有り得ます。

呼ばれる側 SomeProcess()でチェックする場合、それを仕様書や呼び出し規約に明示しないと、
呼び出す方でもチェックして、無駄になります。

呼ばれる側でチェックする場合、NULLチェックに引っかかった事をどうやって呼び出し元に知らせるのか、
も考えないといけないかも知れません。戻り値で返されればいいですが、それ以外の用途に戻り値を使っていると、ちょっと困ります。
そういう場合は、呼び出し元でチェックするのも一つの手と思います。

Answer 6

itemが何なのか、どの程度の範囲で使われてるかも不明ですし、提示されたコードからは判断出来ません。
itemをSomeProcessからしか操作しなくて、SomeProcess側でチェックしているなら呼び出し側でチェックする必要はないでしょうし、SomeProcess内でチェックをしていなくて、かつSomeProcessが自分の触れないコードなら、呼び出し側でチェックするしかないでしょう。

Answer 7

この質問が初歩的だなんてとんでもない。これはベテランでも的確な判断はできないです。こういう点に着目して疑問に思えるということは大事です。素質があります。

以下「私ならどうするか」という立場で回答します。あくまでも一つの意見です。これが正解だとは限りません。

この場合に限っては、私は「メソッドを呼び出す側(Main)に記述」を選択します。なぜならば:

1. SomeProcessはprivateで、Mainも自分で書いているので、自分でルールを決められる。「item==nullのときはSomeProcessは呼び出さない」というルールにして徹底的に守る。
2. メソッド呼び出しには実行時コストがかかる。そのコストを最小化するために、呼び出し側でフィルタする。

ただし他の多くの場合では、私は呼び出される側でnullチェックを記述します。

1. SomeProcessがprivateでない場合など、自分以外が呼び出す可能性があるなら、呼び出される側に記述する。なぜならば「item==nullのときは呼び出さない」というルールを徹底できないから、呼び出される側に書かないといけない。
2. ギリギリの高性能を狙うなら、呼び出し側でもフィルタする。メソッド呼び出しのコストに比べて、nullチェックのコストはだいぶ安い。
3. SomeProcessがprivateでなくても、「item==nullのときは呼び出さない」というルールを徹底できる方法が別にあるなら、呼び出す側に記述することにする、かもしれない。
4. プロジェクトで使うコーディング規約で指定してあるかもしれない。
5. CERT C コーディングスタンダード では API00-C. 関数のなかで引数を検証する よう推奨している。
6. MISRA Cでは、ライブラリに渡す値の妥当性を確認しなければならない とルール化している。
7. Nullable/Maybe/Optionalを使える言語であれば、使うことを検討する。参考: C#でのnull参照問題(後編)

Comments

[as00000]

回答ありがとうございます
参考になる回答をたくさん頂きとても勉強になりました
本当はどれも有用な意見で”ベスト”は決められなかったのですが
中でも詳細に説明いただいたこの回答をベストアンサーとさせて頂きます

皆さんの意見を拝見して、呼び出され側ではチェックするという声が多かった様なので
私も今回のようなケースの場合は呼び出され側では必ずチェックを行なう様にしようと思いました
個人でやっていて仕様書などもないので、しばらくしてからコードを修正する時に
絶対に「内部でチェックしてるんだっけ？」となる事が想定されるので（笑）
呼び出され側できちんと対策してあれば、
呼び出し側でチェックを忘れてもエラーは発生しないと思われた為です
（無駄なチェックは増えますが）

何にしてもこの場合はこう！と決められる内容ではないみたいなのと
「ルールに沿っていれば良い」という意見も結構あったので
もう少し”仕様”に近いものを決めて作業する様にしないといけないなと思いました

皆さんのご回答ありがとうございました