C言語 - 利便性と可読性を求めるための複合文で囲むのはあり?!?
退会済みユーザー
総合スコア0

Question

```
if (cho == 9) {
      for (i = 0; i < 10; i++) {
        {
            int cnt = 0;
            printf("%3d ~ %2d : ", i*10, i*10+9);
            for (j = 0; j < bunpu[i]; j++) {
                cnt++;
            }
            printf("%3d人  ", cnt);
            for (j = 0; j < bunpu[i]; j++) {
              putchar('*');
            }
            putchar('
');
          }
      }

      {
          int cnt = 0;
          printf("   100   : ");
          for (j = 0; j < bunpu[10]; j++) {
              cnt++;
          }
          printf("%3d人 ", cnt);
          for (j = 0; j < bunpu[10]; j++) {
            putchar('*');
          }
      }

  } 
```

プログラムの一部ですが、長くなり、読みにくくなったので、上のように複合文で囲もうと思ったのですが、実行速度、慣習的に問題はないのですか?

Accepted Answer

このあたりは、好き嫌いの問題とか、コーディング規約的な縛りという所だと思います。

一昔前(といってもかなり昔ですが)であれば、例えば、
```
int cnt
```
変数が、２回スタックに積まれて解放されるので、速度が・・・などという話もあったかとは思いますが、今のコンパイラは優秀ですし、そもそも現状ギガで動作するCPUからしたら、どうでもよい処理時間ですので、気にすることはないかと思います。

ただ、やはりある程度意味のある分け方をするなど、読み手が、なんで、分けてるのかな？という事がわかるという事は重要です。

なので、例えば上記では、コードを２分割していますが、それがどのような意味があるのか？という事が重要かと思います。そうゆう意味では、単に長いから・・・ではちょっと、説明としては、よろしくなく、それぞれのブロックにおいて、例えば、
```
{//ほにゃらら処理
 for(){
 }
}
{//ほにゃらら処理だけと、１００だけの場合
 for(){
 }
}
```
とか、コメントをつけるとか、することで、優しい記述となるかと思います。

ただし、もし
```
int cnt;
```
の意味することが、大きく上下で違うようであれば、ブロック分けをせず、素直に別名の変数を使うべきでもあります。（今回はカウントするという意味っぽいので、よさそうですが。）

また、
```
{
}
```
の記述は、もしかしたら、元は、
```
//if(ほげほげ)
{
}
```
のような、もともとif分かなにかあったのを、コメントアウト後で削除したのでは？という、深読みをされる可能性がありますので、
やはり、内故、をコメントしておく事が、きちんとすることが大切かと。

また、上記のようななんとなく似ている処理であれば、関数化できないかどうか、検討することも必要かと思います。

ちなみに、私は関数内部用の変数において、変数スコープを関数レベルからさらに狭める目的で利用し、できるだけ、イージーミスでの、変数の取り違いなどを行わないように、結構多様する人間です。

そして、そうゆう意味で使うときは、int cntA; int cntBと分けてコーディングしますね、、、両方cntのままだと、コメントアウトなどでの、ちょっとした修正で、スコープからはずれてしまっても、わかりませんので。

それなら、複合文いらないんじゃない？となりますが、今度はエンハンスなどでコード追加時にcntAなどが、他で参照されない、保証のために、複合文で囲っておきます。

いわゆる、忘れやすい自分のための、自己防衛です。

Answer

> 長くなり、読みにくく

を解消するためであるならば、関数を分けるとか、コメントを付けることで対応するのが最初と思います。
// ↓↓↓ 処理xxx ここから
・・・・・
// ↑↑↑ 処理xxx ここまで
のようなことをやります。

ローカル変数のスコープが問題なら、役割にふさわしい変数名を付ける
を最初に考えたほうがいいと思います。

他人が見た時になぜ ｛｝で囲まれているのか、すぐにわかるようにしておいて下さい。
その上で使うのは問題ないと思います。

Answer

そのコードは、ようするにcnt = bunpu[i]になるのでは？
そもそもcnt変数は不要だし、cnt++を行っているfor文も不要だと思います。

Answer

この程度なら１画面に収まるのでまだ問題はないと思いますが、行数が増えると見にくくなります。
cntも違う意味で使っているので、ループが長くなって１画面に収まらなくなると勘違いの元になりそうです。

こういう場合は、ループ内の二つのブロックをそれぞれ関数化したほうが良いと思います。
基本的に、１つの関数、ループはできるだけ短くするように、機能ごとにサブルーチン化していくと可読性が上がります。

Answer

ローカル変数のスコープを限定する目的であれば、十分有りだと思います。ただ、スコープを限定しないと見通しが悪くなると言うことは、それだけ処理が結構長く、それぞれ独立しているのでは無いでしょうか？そうであれば、ブロックで囲むことよりも、関数として外出しすることを考えます。その方が、同じコードを再利用できるし、処理の責任分担を明確にできます。速度についても、inline関数にすれば呼び出しコストがかかりませんので、あまり変わりません。

投稿2016/04/22 22:55

raccy

総合スコア21866

Answer

こんにちは。

私はよく使います。
提示されたソースのように`cnt`変数のスコープを限定することで可読性を改善できる時はその１つですね。
特にswitch文のcaseブロックで一時変数を定義したい時に非常に有用です。

また、ブロック内で定義した変数がブロックの最後で自動的に破棄される性質を使って、[RAII](https://ja.wikipedia.org/wiki/RAII)でミューテックスを獲得／開放するような時に重宝します。
ブロック先頭でミューテックスを獲得すればブロック内は排他制御できています。そして、例外が発生しても開放漏れしません。

また、問題になるような性能劣化も無いはずです。（高速化のためにループを展開するようなギチギチな高速化をする時は別ですが。）

ただ、下記記述は個人的には反対です。
```C++
for (i = 0; i < 10; i++) {
  {
    // 中略
    }
}
```
インデントが合ってないです。ま、それは単なるミスと思いますが、これを除いてもforループの範囲がすごく見難いです。
for文については、2重に{}でくくる意味はないように思います。といいますか、特に理由もなく2重にくくっていることが原因でforループの範囲が見難くなっているような気がします。

Answer

可読性を考えてブロックで括るというのは、ありだと思います。ローカル変数のスコープを限定することで、最適化されやすくなるという効果もあります（変数をたくさん使うような場合）。
ただし、関数化して外に出した方が見やすいこともあるので、その辺は状況に応じて判断してください。