昔の記事にも書いたのだが、ソースコードのネストは小さければ小さいほど読みやすい。
ではどうするか。一つは、returnできるところでさっさとreturnするというのが良いだろうと思う。
たとえば、
public int func1(int a, int b)
{
int result;
if (a > 10)
{
if (a > 20)
{
if (a > 30)
result = b;
else
result = 3;
}
else
result = 2;
}
else
result = 1;
return result;
}
これは以下のように書ける。
public int func1(int a, int b)
{
if (a < 10)
return 1;
if (a < 20)
return 2;
if (a < 30)
return 3;
return b;
}
あと、同様の例で良く見るのが、意図したデータが与えられたかどうかのチェックを行うたびにネストさせるケースだ。
public bool isValid(Data a)
{
if(a != null)
{
if(a.Count > 0)
{
if(a.Error == null)
{
if (a.Result != null)
return true;
}
}
}
return false;
}
これは以下のように簡潔に書いたほうが良い。
public bool isValid(Data a)
{
if (a == null)
return false;
if (a.Count > 0)
return false;
if (a.Error == null)
return false;
if (a.Result != null)
return true;
else
return false;
}
また、ループ中で条件を判定するのもネストが深くなりがちだ。
public IEnumerable<Result> GetSuccessedResult(WorkList works)
{
List<Result> results = new List<Result>();
foreach (var w in works)
{
if(w.Successed)
{
if(w.ResultContext != null)
if(w.ResultContext.Score > 95)
results.Add(w.ResultContext.Result);
}
}
return results;
}
C#などで、高階関数やラムダ式などの近代的な機能が使える場合はもっと簡潔に書ける。Java？知らん。さっさとJava 8かScalaに移行しろ。
public IEnumerable<Result> GetSuccessedResult(WorkList works)
{
return works.Where(w => w.Successed)
.Where(w => w.ResultContext != null)
.Where(w => w.ResultContext.Score > 95)
.Select(w => w.ResultContext.Result);
}
または、yield returnなどを使うのもよいだろう。遅延実行されるのが嫌な場合は、最後に.ToList()などを呼んであげると良い。
なぜネストが深くなるのか
ネストを深くする人は自分で書いていて読みにくくないのだろうか。ネストを深くする人は、弾けるケースを最初にはじいていない、スキップできる要素を最初にスキップしていない、という印象を受ける。
複雑な条件分岐がネストしてしまう場合は、わざわざ難しい方法や順序で分岐判定を行っていないかちょっと考えてみるべきだ。私はC#で数年間、毎日毎日コードを書いてきたが、ネストは深くても2つであるケースが９割くらいだと思う。3段になると嫌気が差してきて2段以下にしようと書き直す。4段以上はほとんど書かない。どうしても避けられず、かつ、エディタ上の一画面程度で終わってしまう行数のメソッドなどならばそのように書いたこともある。
関数の途中でreturnするのは行儀が悪い？
新人の頃、関数の途中でreturnするようなコードを書いていたら「それは行儀が悪い」と怒られたことがある。曰く、「これは関数である。関数はただ一つの出力を取る。それなのに、あちこちでreturnして色々な値を取っていたらコードが追えない。値の出力をするところ、すなわちreturn節は関数の最後に一つだけ書くべき」と言われた。
確かに、私も昔読んだ本で上記のような主張は聞いたことがある。しかし、個人的には到底承服できない。
理由その1。return節が複数あるのが悪いなら、下記の数式の表現だって悪い書き方だと言えるのではないか。しかしこの書き方に異を唱える人などいないだろう。
理由その2。returnをあっちこっちに書くな、というのは、GOTO文が多用されてたころの文化の名残ではないか。return、continue、breakなどはGOTO文と同じJump命令であるが、戻る場所が明確にわかる制限されたJump命令である。GOTO文が主流だった時代の常識は現代に通用しない。
理由その3。returnを最後に書くべきだったのならば、なぜ、CやC#やJavaやその他数多の言語使用を策定するときに、「returnは必ず一つ」という制限を入れなかったのだろうか。これはつまり、ほぼすべての人がreturnは複数個書くことを想定していて、かつ、言語仕様を最初に策定した偉い言語屋さんもそれを良しと思っていたからである。
理由その4。これは厳密には「関数」じゃない。構造化プログラミング言語であれ、オブジェクト指向言語であれ、「関数」と呼ばれる一連の手続きは副作用を含む。関数とは入力から出力への射である。マップである。集合論的に言えば二項関係である。射にも二項関係にも、グローバル変数やインスタンスのフィールドを変えるような操作は定義されていない。そもそも関数じゃないのに「関数だから出口は一つ」と言ってるのがおかしい。「これは関数なんだから～」とかドヤ顔で解説し始めるならHaskellとかF#を使えばよろしい。
結論
ネストは浅くあるべき。要らん物、もしくは、考えないケースは最初でガンガン切り捨てるべき。
（補足）今回の記事は、「なぜプログラムのコードは複雑になっていくんだろう。 - かずきのBlog」を読み、まったくその通りだよなあ。やっぱ俺の考えは間違ってないよなあ。と思って書きました。
いつも楽しく読ませていただいております！
ネストについて同感です。
当方も昔はreturnは関数最後に一個派でした・・・(^^;
蛇足ですが、以下のコードが異なっている気がします。
if (a < 20)
return 1;
重箱のスミで申し訳ありません。
ご指摘ありがとうございます。修正しました。
多分書きながら何か間違ってるだろうなとは思いました･･･。
ネストの深いコードは読み間違えやすい実例だったということにしておきましょう。
（本当はコピペして修正し忘れただけ）
私も入社当時はこのように指摘されたので従ってましたが、
数か月で辞めた記憶があります。
ソフト開発も流儀や文化がいろいろあって難しいですね。
http://www.ailight.jp/Blog/kazuk/archive/2005/3/10/構造化プログラミングについてのテスト
http://www.ailight.jp/Blog/kazuk/archive/2005/3/11/構造化プログラミングについてのテストの正解と解説
↑を読むと面白いかも
ありがとうございま。読んでみます。
[…] コードのネストを深くするな | anopara昔の記事にも書いたのだが、ソースコードのネストは小さければ小さいほど読みやすい。 ではどうするか。一つは、returnできるところでさっさとretur […]
[…] - – - – - – - – - – - – - – - – - – - - コードのネストを深くするな | anopara […]
[…] コードのネストを深くするな | anopara 昔の記事にも書いたのだが、ソースコードのネストは小さければ小さいほど読みやすい。 ではどうするか。一つは、returnできるところでさっさとretu […]
ネストを浅く保つ件については全くその通りなんだけど、「リーダブルコード」や「クリーンコード」を読まないと中々問題に気づかないと思う。
まあ殆どの問題はテスト駆動開発で解決するんだけどね。（テストを先に書くのだから、テスト不可能な深いネストなんて最初から書かない）
テスト駆動開発だと自然と解決するとのこと、同意します。
残念ながら実業務ではそのような開発方法を見たことがありませんが…。(テストドリブンへの理解が得られない)
そうですか？僕は新しい職場に移ったばかりですけど、周りをテスト駆動開発に巻き込みましたよ。CIの仕組みも作ったので、コミットからrpm作成まで全部自動です。
行動で示せば、分かってくれる人も居るのではないでしょうか。
テスト開発駆動をするなと言い出す職場ならさっさと逃げ出すのが正解でしょう。
そうして皆を巻き込んでいい方向に引っ張っていける人は尊敬します。
私の場合、言い出すものの途中で反対に合うと面倒になってしまう意志の弱さがあるので。
別の職場に逃げるのも何度も考えつつ結局できずにいます。
[…] コードのネストを深くするな | anopara […]
修正後の if 条件は if (a <= 10) だと思います。
以下同様。
ご指摘ありがとうございました。よくよく見ると他にも間違いがありますね。修正します。
[…] コードのネストを深くするな | anopara […]
「return文を一つに」というのは「関数の戻る場所を一つにする」の勘違いですよね
昔の言語だと「関数スタック2つ分戻る」とかいう事が出来たので
「関数を呼んだら呼んだ場所に戻ってくることが保証される」という意味で
なるほど！上のコメントでも参考ページを教えてくれた人がいましたが、スタック2つ戻る事が出来たのですね。
C言語から入った私には使いどころが想像つきませんが…。
[…] コードのネストを深くするな | anopara 昔の記事にも書いたのだが、ソースコードのネストは小さければ小さいほど読みやすい。 ではどうするか。一つは、returnできるところでさっさとretu […]
これずっと思ってた。
forとかwhileだけならまだしも、ifとかswitchとかで巨大化してて
どのスコープにあるのか確認するのめんどいことがよくある。
それで間違ったとこにelse書いてあったりとか。
アンチgotoカルトなんかもそうだけど、もとはバグを抑えるための
流儀だったはずなのが形ばっか拘ってバグ増やしてちゃ逆効果
ってことで。
＞間違ったとこにelse書いてあったり
これ私も仕事でよく見ます！書いた人はこれでイライラしないのか不思議でした。
この記事は私も長年疑問に思っていたのですが、あまりそういうことを主張する人はいなかったので書きませんでした。
書いてみたら思いの外反響があってちょっとびっくりしています。
最近はIDEの補助機能がしっかりしてるから、スコープで迷ってキツイということもないだろう。
ネストなんかで文句言ってたらオープンソース読めないだろ。
まあそうですね。
インデントが深いとターミナルでソースが読みにくいからオープンソースのインデントはむしろ浅いのが多いんじゃないのか？
ちょっとネストが深いぐらいでｇｄｇｄ言ってるのはヘタレの証。
条件のチェックごとにリターンする方法も場合によってはスッキリかけるが、ネストさせて、次の条件式までにいろいろ計算したい時もある。そういった思考を表現するにはネストも有り。
＞次の条件式までにいろいろ計算したい時もある。
そういう場合ならネストが深くなるのも当然OKですよ。とは言っても2-3階層程度の話ですけどね。それ以上深くなるなら新しい関数を書いて呼び出した方が良い。
[…] コードのネストを深くするな | anopara 昔の記事にも書いたのだが、ソースコードのネストは小さければ小さいほど読みやすい。 ではどうするか。一つは、returnできるところでさっさとretu […]
> 関数の途中でreturnするのは行儀が悪い？
私は、関数内でreturnするパターンは大間かに以下の2つあると思っています。
A. 関数の先頭でreturnするパターン(主ロジックに入る前)
int f(){
int x;
if(...) return 1;
if(...) return 2;
// 主ロジック
for(...){
for(...){
if(...){
// ...
}
}
}
return x;
}
B. 関数の途中でreturnするパターン(主ロジック内部)
int f(){
int x;
// 主ロジック
for(...){
for(...){
if(...) return 1;
if(...){
// ...
if(...) return 2;
}
}
}
return x;
}
「関数の途中でreturnするのは行儀が悪い」というのは「主ロジック内でreturnするのは行儀が悪い」を意味しているのでないでしょうか。
Aのパターンは主ロジック前なのでOKなコードで、Bのパターンは主ロジック内でreturnしているのでNGなコードではないかと私は思います。
これはあくまで私の考え方なので、参考になればと思って投稿してみました。
コメントありがとうございました。
この2つを比較するとAのほうが綺麗に見えますね。Bのようにループに入ってからreturnされると確かに気持ち悪いです。
ただ、ちょっと違う話かもしれませんが、線形探索のようなことをする場合はBのようにしかかけない場合もあるのかなとも思いました。
厳密に言うならばそれぞれのコードを実際に見なければ判断できないことだとは思いますが。
[…] コードのネストを深くするな | anopara […]
[…] コードのネストを深くするな […]