わさっきhb

大学(教育研究)とか ,親馬鹿とか,和歌山とか,とか,とか.

while(1)のループとその除去について〜FizzBuzz問題を例に

いきなりですが問題です.

3つの正整数a,b,cを入力にとり,1からaまでを1ずつ順に出力しなさい.ただし,bで割り切れる場合は「Fizz」を,cで割り切れる場合は「Buzz」を,両者で割り切れる場合は「Fizz Buzz」を,該当する数の代わりに出力すること.

FizzBuzz問題wikipedia:Fizz_Buzz)では,bとcの代わりに3と5が使われますが,ここでは変数としておきました.
さっそくですが解答として,ある学生が,以下のようにプログラムを書いたら,どのようにアドバイスしましょうか.

#include <stdio.h>

int main(void)
{
  int a, b, c;
  int number = 1;

  scanf("%d", &a);
  scanf("%d", &b);
  scanf("%d", &c);

  while(1) {
    if (number % b == 0) {
      printf("Fizz");
      if (number % c == 0) {
        printf(" Buzz");
      }
      printf("\n");
    } else if (number % c == 0) {
      printf("Buzz\n");
    } else {
      printf("%d\n", number);
    }
    number++;
    if (number > a)
      break;
  }

  return 0;
}

コンパイルで引っかかることなく,実行プログラムができました.いくつか入力を与えてみると,仕様どおりに出力してくれます*1
入力のとり方ですが,文字列ではなく数値ですので,scanfは簡便かつもっとも有望な方法と言っていいでしょう.なお,コマンド実行のたびに入力値を与えるのは手間ですので,

echo 20 3 5 | ./fizzbuzz

とすれば,コマンドラインを使って指定できます.繰り返し実行や,入力値を一部変えるのも,手軽に行えます.
さて,動作はいいとして,コードのほうをきちんと見ていきましょう.すると保守性の観点で,いくつか,まずいものが浮かび上がってきます.具体的には:

  • scanfを3回に分けて呼び出しており,しかも適切に読み出されたかのチェックをしていません.
  • 反復(ループ)を,「while(1)」による無限ループで記述しています.
  • 数がcで割り切れるかのチェックを,2箇所で記述しています.

先に,これらの課題を解消したコードをお見せします:

#include <stdio.h>

int main(void)
{
  int i;
  int a, b, c;

  if (scanf("%d %d %d", &a, &b, &c) != 3
      || a <= 0 || b <= 0 || c <= 0) {
    printf("wrong number\n");
    return 1;
  }

  for (i = 1; i <= a; i++) {
    if (i % b == 0 && i % c == 0) {
      printf("Fizz Buzz\n");
    } else if (i % b == 0) {
      printf("Fizz\n");
    } else if (i % c == 0) {
      printf("Buzz\n");
    } else {
      printf("%d\n", i);
    }
  }

  return 0;
}

先ほどの「まずいもの」は,順に次のとおり解決しました.

  • scanfは1回の呼び出しで3つの変数に値を格納させるようにしました.読み出せた個数と,それぞれの値を検査し,不適切ならそこで終了としました.
  • ループについては,「1からaまで」ですので,for文に置き換えました.
  • 割り切れるかのチェックは「bとcでともに割り切れるか」「bのみで割り切れるか」「cのみで割り切れるか」の3つにし,条件判定はif〜else if〜else if〜elseのフラットな構成にしました.*2

今回の元ネタは,所属する学科の教員の問い合わせです.「2年のCのプログラミング課題で,反復はwhile(1)と無限ループを書いて,抜け出す条件(ifとbreak)をその内部に書いているコードを見かける」「インターネットの情報を元にしているらしい」「良くない書き方であることを,1年後期の科目で指導できないか」という意見があったのでした.
「反復と言えばwhile(1)やfor(;;)」,という習慣を持つのは確かにいただけませんが,無限ループとif+breakを使うことで,意図が素直に書けることもあるように思います.したがって「while(1)やfor(;;)を使うな」というわけにもいきません.何をしていい(you can),何をしなければならない(you must),といったことについては,Cの本をもとに今年2月,記事を書いたのでした.
とはいえ,あなたが学生であれば教員が,仕事に就けば上司が,書いたコードをレビューしてくれる機会は大事なものです.さらに言うと,コードを書いたあとの自分というのも,大事な(そして教師・上司よりも前の)レビュアとなるわけで,見てもらう前の手直しも,できれば行いたいものです.
ループに書き方について,基本ルールを箇条書きにしてみます:

  • 反復の上下限などが決まっているもの(たいていの数値処理)はfor,決まっていないもの(たいていの文字列処理)にはwhileを使います.
  • while(1)やfor(;;)による無限ループは,本当に必要か,意図を素直に記述したものかを考えます.
  • 繰り返し条件が複数になる場合は,ループの入れ子にすべきかどうかをまず検討します.単ループで複数の繰り返し条件がある場合には,whileやforの条件式で(&&などを用いて)書けるかをまず判断し,それが困難なら,whileやforによる条件判定と,反復の中での条件判定(if+break)を書きます.それぞれコメントもつけます.

見直しに関連して,「レッド・グリーン・リファクタリング」という言葉も,自習しておくといいでしょう.これはテスト駆動開発(test-driven development, TDD)では当たり前の概念です.1〜2年のCプログラミングでTDDを行えとは言いませんが,一つのプログラミングスタイルとして,知っておいて損はないと思います.いや,これは,10月からの講義で取り上げましょう!

(最終更新:2013-09-02 深夜)

*1:今回は「仕様どおりに動作するプログラムを,より良くする」ことを主眼としていますが,仕様どおりに動作しないパターンとして,「両者で割り切れる」を「a % (b * c) == 0」と書くというのが考えられます.そうしてしまうと,a, b, cにそれぞれ13, 4, 6を代入したら,12のときに「Fizz Buzz」と出力してくれません.「a % (bとcの最小公倍数) == 0」とすべきですが,この規模のプログラムで,最小公倍数を求める必要はないでしょう.もし剰余演算が高コストなら,bの余りとcの余りに対応する整数型変数を別に確保し,1を加えたりリセットしたりすればいいのです.

*2:追記:「数がcで割り切れるかのチェックを,2箇所で記述しています.」の解消をしていませんでした.変更後のほうが,処理系・実行環境次第で(i % c == 0の判定を2度行うことがあり)時間がかかるようになってしまうかもしれません.わずかな時間よりも,読みやすさを優先しました.別の言い方をすると,同じ条件判定を2箇所で記述するのは冗長に見えたけれど,整理して書き直したところ,そこは取り除かないほうがよいと判断した,という次第です.