LoginSignup
381
289

More than 5 years have passed since last update.

@DeployCat(Yuho Miki)

ネストの深さは闇の深さ

Last updated at Posted at 2016-11-10

はじめに

こんにちは、先日投稿したソースコードの混沌はどこから這い寄るのかという記事が思いのほかストックされて浮かれ気味のMic-Uです。

そのときは「こうしようぜ！」と訴えるだけでしたので、今回は実際に戦った過程を紹介したいと思います。

今回扱うのはこれまた仕事で遭遇したカオスコードですが、先日の記事の投稿後に新たに発見されたものです。
その特徴は簡潔に言うとやたらとネストが深い。
ネストが深いのはカオスコードにはよくあることです。

ネストの深さがもたらす混沌

プログラミング経験者なら誰もが「ネストは深くしてはいけない(戒め)」と言われたことがあると思います。
本当に始めたばかりでそんなこと言われたこと無いという人も、いつか必ず言われます。
ていうか今私が言ってます。

戦いの記録に入る前に、改めてネストが深くなるとなぜいけないのか見つめ直しましょう。

物理的問題

ネストが深くなると視覚的に構造が把握しづらくなります。
私は過去にネストが深すぎて、横スクロールしないと画面が真っ白になるコードを見たことがあります。

そのとき使ってた言語はC++でしたが、

khaos_nest.cpp

if(a == 1){
    if(b == 2){
        if(c == 3){
            if(d == 4){
                if(e == 5){
                    if (f == 6){
                        //こんなのがずっと続く
                    }
                }
            }
        }
    }
}

再現してみたかったのですが、めんどくさいのでやめました。
とにかく、横スクロールしないと見えないコードというのは非常に辛いです。

真っ当なエディタやIDEを使っていれば、右の方に縦線が一本走っています。
個人的にはこれは超えてはいけないラインだと思っています。

脳内メモリが足りなくなる

ネストの構造を把握するには以下の情報を脳内で処理する必要があります。

  • 「このスコープに入るのはどういった条件のときか」
  • 「このスコープはどこまで続いているか」

ネストが深くなるというのはつまり、ある条件下で入ったスコープの中で、さらに特定の条件下で一層下のスコープに入るということで、
脳内メモリに保っておくべき情報が倍々ゲームで増えていきます。

さらに、これは視覚的問題にもつながりますが、ネストが深くなるにつれてスコープがどこまで続くかもわかりにくくなります。
↑に上げたコードもこれぐらいならまだなんとかなりますが、この調子で続けられて縦横スクロールを駆使しないと見えないとかなったらあまりにも辛いです。
これによって脳への負荷はさらに増大します。

エディタやIDEが「この"{"はこの"}"と対応してるんやで」と教えてくれますが、あまりそれに甘えるのも考え物です。

どの程度までなら許容できるか?

ネストは浅い方がいいのですが、現実的に考えて多少は使わざるをえません。
個人的には二重か三重ぐらいが常人がすぐに構造を把握できる限界ではないかと思います。
少なくとも自分はあまり頭が良くないのでそれ以上になると脳内メモリが足りなくなってストレスで寿命がマッハです。

「五重でも六重でも余裕、その気になればもっといける」という超人も世の中にはいらっしゃるかもしれませんが、
そういった方々にとっても脳への負荷は軽いに越したことはないと思います。

そして戦いの幕が上がる

それではご覧いただこう。
こちらが今回私が戦ったカオスコードである。

khaos_before.php
function get_tbl_data_mapping_info($db, $table_id, $disp_mode=0, $org_no=null){
  //レコード取得
  $result = select_tbl_data_mapping_info($db, $table_id, $org_no);

  if ($disp_mode != 0){
  //表示調整(○○○○CD の場合に、前の文字数が長いために「CD」が表示されなくなるのを対処
    for ($i=0;$i<count($result);$i++){
      $pos = strpos($result[$i]['name'],"CLASS_CD_");
      if ($pos === false){
      } else {
      //区分CDの場合('CD'が最後に現れる箇所)
        // $result[$i]['disp_name']はシステム側で自動生成 末尾が「CD」の文字列か空文字列
        $pos = strrpos ($result[$i]['disp_name'],"CD");
        if ($pos === false){
        } else {
          //文字列CDが有る場合
          $f_name = substr($result[$i]['disp_name'], 0, $pos);
          if (mb_strlen($f_name) > 4){
          //4文字以上の場合は5文字目以後を切り落とし
            $f_name = mb_substr($f_name, 0, 4) . "..CD";
          } else {
            if ($f_name != ''){
              $f_name .= 'CD';
            } else {
              $f_name .= '　';
            }
          }
          //上書き
          $result[$i]['disp_name'] = $f_name;
        }
      }
    }
  }
  return $result;
}

こまかい仕様の話は今回の話の趣旨から外れるので割愛しますが、
私はこれを見るだけで辛くなりました。

深い...
ifがどこまで続いてるかわからない...
どげんかせんといかん...!

それでは戦いの過程をディープネストコードあるあるに乗せて追っていきましょう

あるある①　大きなのっぽのifスコープ

注目してほしいのはif ($disp_mode != 0)ってところ。
この条件に当てはまったらネストを掘り進むことになるのですが、当てはまらなかったらすぐにreturnされます。
そのくせifのスコープはずっと続いているので、上から順に読んでると「$disp_mode == 0だったらどうなるんや」という考えが脳内メモリの一部を使い続けます。
しかし最後までその時は訪れません。
脳内メモリの無駄遣いです。ストレスです。

ですので、まずはアーリーリターンでこいつを排除します。

function get_tbl_data_mapping_info($db, $table_id, $disp_mode=0, $org_no=null){
  //レコード取得
  $result = select_tbl_data_mapping_info($db, $table_id, $org_no);
  if($disp_mode == 0){
    return $result;
  }

  //表示調整(○○○○CD の場合に、前の文字数が長いために「CD」が表示されなくなるのを対処
  for ($i=0;$i<count($result);$i++){
    /** 省略 **/
  }

  return $result;
}

１つネストが浅くなりました。
アーリーリターンのおかげで、$disp_mode == 0の場合のことはこれ以降もう考えなくていいのです。
これで脳内メモリを少し解放できてストレス軽減になります。

あるある② 数珠つなぎの条件

何を言ってるかというと

if($a == 1){
  if($b == 2){
    // 何らかの処理
  }
}

こうやって繋げていくのはやめて


if($a == 1 && $b ==2){
  // なんらかの処理
}

こんな感じでできるだけまとめようよ、という話です。
上記のカオスコードでいうと

$pos = strpos($result[$i]['name'],"CLASS_CD_");
if($pos === false){
} else {
  //区分CDの場合('CD'が最後に現れる箇所)
  $pos = strrpos ($result[$i]['disp_name'],"CD");
  if ($pos === false){
  } else { //省略

この辺。

if(){}の中身が空のままelse{}の中で処理をしたり、
論理的に別物のはずなのに$posという変数を使いまわしたりしていることがわかりにくさを助長していますが、
歯を食いしばって私は戦います。

このカオスコードを丹念に読み解けば、
strpos($result[$i]['name'],"CLASS_CD_") === false
または
strrpos ($result[$i]['disp_name'],"CD") === false
の時は何もせずに次のループに進むことがわかります。
continue;を使わなきゃ...!(使命感)

戦いの果てにfor文の中身はこうなりました。

for ($i = 0; $i < count($result); $i++){
  //表示調整(○○○○CD の場合に、前の文字数が長いために「CD」が表示されなくなるのを対処
  //項目名が「CLASS_CD_hoge」かつ表示名が「fugaCD」の場合のみ処理を行う
  // $result[$i]['disp_name']はシステム側で自動生成 末尾が「CD」の文字列か空文字列
  $class_cd_pos = strpos($result[$i]['name'],"CLASS_CD_");
  $cd_pos = strrpos ($result[$i]['disp_name'],"CD");
  if ($class_cd_pos === false || $cd_pos === false){
    continue;
  } 

  //文字列CDが有る場合
  $f_name = substr($result[$i]['disp_name'], 0, $cd_pos);
  if (mb_strlen($f_name) > 4){
  //4文字以上の場合は5文字目以後を切り落とし
    $f_name = mb_substr($f_name, 0, 4) . "..CD";
  } else {
    if ($f_name != ''){
      $f_name .= 'CD';
    } else {
      $f_name .= '　';
    }
  }
    //上書き
  $result[$i]['disp_name'] = $f_name;
}

なんだ、そんなに複雑な条件じゃないじゃないか!!
少し光が差してきたぞ!

おまけ

条件が入り混じってて、まとめたら逆に辛いっていう場合もありますが、
そういう時には「これってつまりどういうことなの?」ということを考えてisHoge()みたいなメソッドにするのがいいと思います。

例えば

  1. 脊椎動物である
  2. 恒温動物である
  3. 卵生である

これら全てを満たすものって言われるとよくわからないけど、これはつまり

  • 鳥類である

ってことですね。
なのでisBirdみたいなメソッド作って、上記1,2,3を満たしていたらtrueを返すみたいにすれば良いかな、と。

あるある③　メソッドが分けられてない

よくありがちなパターンとして、forやwhileのループの中でさらにifを使った込み入った処理をしてて、
その結果ネストが深くなるというパターン。

今回私が注目したのはここ

$f_name = substr($result[$i]['disp_name'], 0, $cd_pos);
if (mb_strlen($f_name) > 4){
//4文字以上の場合は5文字目以後を切り落とし
  $f_name = mb_substr($f_name, 0, 4) . "..CD";
} else {
  if ($f_name != ''){
    $f_name .= 'CD';
  } else {
    $f_name .= '　';
  }
}

これはこれでひとまとまりっぽいし、分けていいと思うんすよねぇ...
こんな感じに

function shorten_f_name($f_name){
  if (mb_strlen($f_name) > 4){
  //4文字以上の場合は5文字目以後を切り落とし
    $f_name = mb_substr($f_name, 0, 4) . "..CD";
  } else {
    if ($f_name != ''){
      $f_name .= 'CD';
    } else {
      $f_name .= '　';
    }
  }

  return $f_name;
}

このぐらいならあまりありがたみがないけど、アーリーリターンを徹底するなら

function shorten_f_name($f_name){
  if (mb_strlen($f_name) > 4){
    //4文字以上の場合は5文字目以後を切り落とし
    return mb_substr($f_name, 0, 4) . "..CD";
  }

  if ($f_name != ''){
    $f_name .= 'CD';
  } else {
    $f_name .= '　';
  }

  return $f_name;
}

戦いの果てに

それでは最終的なコードと元のコードを見比べてみましょう

khaos_before.php
function get_tbl_data_mapping_info($db, $table_id, $disp_mode=0, $org_no=null){
  //レコード取得
  $result = select_tbl_data_mapping_info($db, $table_id, $org_no);

  if ($disp_mode != 0){
  //表示調整(○○○○CD の場合に、前の文字数が長いために「CD」が表示されなくなるのを対処
  // $result[$i]['disp_name']はシステム側で自動生成 末尾が「CD」の文字列か空文字列
    for ($i=0;$i<count($result);$i++){
      $pos = strpos($result[$i]['name'],"CLASS_CD_");
      if ($pos === false){
      } else {
      //区分CDの場合('CD'が最後に現れる箇所)
        $pos = strrpos ($result[$i]['disp_name'],"CD");
        if ($pos === false){
        } else {
          //文字列CDが有る場合
          $f_name = substr($result[$i]['disp_name'], 0, $pos);
          if (mb_strlen($f_name) > 4){
          //4文字以上の場合は5文字目以後を切り落とし
            $f_name = mb_substr($f_name, 0, 4) . "..CD";
          } else {
            if ($f_name != ''){
              $f_name .= 'CD';
            } else {
              $f_name .= '　';
            }
          }
          //上書き
          $result[$i]['disp_name'] = $f_name;
        }
      }
    }
  }
  return $result;
}

これが

khaos_after.php
function get_tbl_data_mapping_info($db, $table_id, $disp_mode=0, $org_no=null){
  //レコード取得
  $result = select_tbl_data_mapping_info($db, $table_id, $org_no);

  if ($disp_mode == 0){
    return $result;
  }

  for ($i = 0; $i < count($result); $i++){
    //表示調整(○○○○CD の場合に、前の文字数が長いために「CD」が表示されなくなるのを対処
    //項目名が「CLASS_CD_hoge」かつ表示名が「fugaCD」の場合のみ処理を行う
    // $result[$i]['disp_name']はシステム側で自動生成 末尾が「CD」の文字列か空文字列
    $class_pos = strpos($result[$i]['name'],"CLASS_CD_");
    $cd_pos = strrpos ($result[$i]['disp_name'],"CD");

    if($class_pos === false || $cd_pos === false){
      continue;
    }

    $f_name = substr($result[$i]['disp_name'], 0, $cd_pos);
    //上書き
    $result[$i]['disp_name'] = shorten_f_name($f_name);

  return $result;
}

function shorten_f_name($f_name){
  if (mb_strlen($f_name) > 4){
  //4文字以上の場合は5文字目以後を切り落とし
    return mb_substr($f_name, 0, 4) . "..CD";
  } 

  if ($f_name != ''){
    $f_name .= 'CD';
  } else {
    $f_name .= '　';
  }

  return $f_name;
}

こう

細かい仕様の話をスルーしてるので「結局何してるかわからねぇよ!」となるかもしれませんが、
見た目の雰囲気はどうでしょう?
個人的には許容できる範囲までスッキリできたのではないかと思ってます。

新人さんへ~結びに代えて~

新人さんは他人が書いたコードを見て理解できない場合、ついつい「自分にスキルがないからだ」と考えがちかと思います。
しかし、個人的には読み手が頑張らなきゃいけないのは以下の2つの場合ぐらいかなと思います。

  1. 使用されている言語の基本的な文法が身についていない
  2. 使用されているライブラリやフレームワークの基本的な作法が身についていない

そうでなければかなりの割合で書いた方の問題だと思いますし、少なくとも書き手はそういう気持ちでいるべきだと私は考えます。

ですので、よくわからないコードに出くわしても自分を責めずに、むしろ「こんなのわからねぇよ」という気持ちを大事にしてください。
わからないということがわかっているのはとても尊いことです。

なんかいい話っぽくなって恥ずかしいので、この辺で失礼します!

381
289
5

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
381
289