【初心者必見】綺麗なコードを書く

目的

FizzBuzz問題を使用して、綺麗なコード・保守性の高いコードとはどういうコードなんだろうか？というところを学びます。

はじめに

プログラムは、書く時間よりも読む時間の方が多いと言われています。
読む時間が多いということは、コードが読みにくいだけで多くの工数を消費してしまう可能性があります。
読みにくいコード、は以下のような状態になっていることが多いです。

1. ifやforのネストが深すぎる
2. 変数名から中身に何が入っているかわからない
3. 関数、メソッド名からそれがどういう仕事をしてどういう結果が返ってくるのかがわからない
4. マジックナンバーの使用
5. 記述方法が統一されていない

読みやすいコードを実現するための第一歩として、「5の記述方法の統一」を行うため、前提として、以下のコーディング規約を先に提示しておきます。

・インデント 半角4つ
・ifやforの後、波括弧の前は、1つ空ける 。if (...) {、for (...) {
・if等の制御構文の括弧の内側は空けない。if (is_null($value)) {
・文字列は文字列展開をしない限り、シングルクォートで囲む。
・変数名を列挙する際は、=を揃える

※会社や個人によって違う部分もあると思いますが、その辺りは適宜置き換えでお願い致します。

FizzBuzz問題

さて、本題です。
プログラムを勉強した人なら一度はやったことがあるであろう問題です。

問題の内容としては、

1から100までの数字を表示する。ただし、ある3の倍数の時は'Fizz'、5の倍数の時は'Buzz'、公倍数の時は'FizzBuzz'と表示せよ。

というようなものです。倍数などの数字は任意です。

上記の要件であれば、望む結果は以下のようなものです。

1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzz
.
.
.
FizzBuzz
91
92
Fizz
94
Buzz
Fizz
97
98
Fizz
Buzz

普通に書く

では、実際にコードを書いていきます。
先に示すのは、普通に動くコードです。

<?php
// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // 3かつ5の倍数
    if ($i % 15 === 0) {
        echo 'FizzBuzz';
    }
    // 3のみの倍数
    elseif ($i % 3 === 0) {
        echo 'Fizz';
    }
    // 5のみの倍数
    elseif ($i % 5 === 0) {
        echo 'Buzz';
    }
    // 3もしくは5の倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

一見悪く無いように見えます。FizzBuzz問題で検索した際に、多くのサイトでこのような書き方が記載されています。
至極単純なコードなので、中身の説明まではしません。

では、修正部分を１つずつ見ていきます。

修正1:なるべく文章の通り読めるようにする

まず、条件式ですが、($i % 3 === 0)の部分に着目してください。
これは、「$iを3で割った余りが0であれば」という意味を表しています。
であれば、この文章の通りにしてあげた方が、ベターと言えます。

「$iを3で割った余り」が主語になるので、その部分を括弧で括ってあげると良いと思います。
$i % 5も同じくです。

    // 3のみの倍数
    elseif (($i % 3) === 0) {
        echo 'Fizz';
    }
    // 5のみの倍数
    elseif (($i % 5) === 0) {
        echo 'Buzz';
    }

修正2：仕様変更に耐えうる記述をする1

次に、仕様変更があった場合について考えてみます。

現在では、3の倍数と5の倍数という仕様になっています。
では、この3や5に仕様変更があった場合どうなるでしょうか。
もう一度プログラムを見てみます。

<?php
// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // 3かつ5の倍数
    if (($i % 15) === 0) {
        echo 'FizzBuzz';
    }
    // 3のみの倍数
    elseif (($i % 3) === 0) {
        echo 'Fizz';
    }
    // 5のみの倍数
    elseif (($i % 5) === 0) {
        echo 'Buzz';
    }
    // 3もしくは5の倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

このプログラムだと、
・15
・3
・5
の3箇所を修正する必要があります。
しかし、これは工夫次第で2箇所に抑えることができます。

修正すると、以下のようになります。

<?php

$a = 3;
$b = 5;

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $aかつ$bの倍数
    if (($i % ($a * $b)) === 0) {
        echo 'FizzBuzz';
    }
    // $aのみの倍数
    elseif (($i % $a) === 0) {
        echo 'Fizz';
    }
    // $bのみの倍数
    elseif (($i % $b) === 0) {
        echo 'Buzz';
    }
    // $aもしくは$bの倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

これで、＄aと＄bを修正するだけで、仕様変更に対応することができました。

追記
@fujitanozomuさんより、ご指摘があったように、＄aおよび＄bが互いに素である必要があるため、＄a = 4, ＄b = 6などの場合は正常に動作しません。そのため、※の部分で＄aの倍数であるかと＄bの倍数であるかというのを分ける必要があります。
@fujitanozomuさん、ご指摘ありがとうございました。

    if (($i % ($a * $b)) === 0) {
        echo 'FizzBuzz';
    }

↓

    if (($i % $a) === 0 && ($i % $b) === 0) {
        echo 'FizzBuzz';
    }

修正3：変数の中に何が入っているのかがわかる名前をつける

先の修正で追加した＄aと＄bという変数は、変数名として相応しくありません。なぜなら、中身が何なのか全くわからないからです。
変数名は、forなどのループカウンタで使う、＄i、＄j、＄kなどは特別として、基本的に一文字はNGです。
実際、実務でもこういったコードを書くプログラマは少なくありません。
$aや$xみたいな変数名は極端ですが、$dataのような変数名が意図なく使われていることがよくあります。

変数名に何が入っているのかがわからないと、最悪それまでのプログラムをずっと遡っていかなければならない、なんてこともあり得ます。
これを考慮して、変数名を変更します。

<?php

$fizz_num = 3;
$buzz_num = 5;

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $fizz_numかつ$buzz_numの倍数
    if (($i % $fizz_num) === 0 && ($i % $buzz_num) === 0) {
        echo 'FizzBuzz';
    }
    // $fizz_numのみの倍数
    elseif (($i % $fizz_num) === 0) {
        echo 'Fizz';
    }
    // $buzz_numのみの倍数
    elseif (($i % $buzz_num) === 0) {
        echo 'Buzz';
    }
    // $fizz_numもしくは$buzz_numの倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

このようにすることで、中身に何が入っているのかが分かりやすくなりました。
変数名を付ける時は、以下のルールに従って命名すると、わかりやすい変数名になると思います。

・スネークケースで単語を区切る

・単数形と複数形を意識する
（例えば、トランプを格納する配列であれば、$card = []ではなく、$cards = []）

・countとnumを使い分ける
（トランプを3人で行っていれば、$player_count = 3）
（トランプを引く順番を管理したければ、$player_num = 1）

・真理値にはisを使う

・長くなっても構わないので、重要な単語は必ず使う

修正4：変数名に使用する単語は略さない

修正3で、$fizz_numや$buzz_numという名前をつけました。
確かに、何の値が入っているのか、明確になりました。
ですが、numとは何のことでしょうか？おわかりの通り、numberの略です。
しかし、本当にそうでしょうか？
numという単語があるかも知れませんし、なにか別の単語の略語かも知れません。実際のところ、読み手がnumという単語を見てnumberである、と認識するのは、推測でしかありません。
確かに、プログラムの世界では、countをcntにしたり、stringをstrにしたりと、これはこの略語だよね、というのがある程度は存在します。これはプログラムを書くチームが共通して認識していれば、問題はないかも知れません。
しかし、略語をOKとすると、同じ単語でも、ある人は略して、ある人は略さない、ということが起き得ます。そうすると、コード全体の統一感がなくなり、読み手に取って、非常に読みにくいコードに仕上がってしまいます。
かと言って、この単語は略して、この単語は略さない、というようなルールを決めるのも非常に面倒で、工数が掛かることです。また、忘れられてしまうことも多いでしょう。
そういったミスや面倒ごとを避けるためには、略さない、というルール一つ決めておけば良いだけのことです。

<?php

$fizz_number = 3;
$buzz_number = 5;

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $fizz_numberかつ$buzz_numberの倍数
    if (($i % $fizz_number) === 0 && ($i % $buzz_number) === 0) {
        echo 'FizzBuzz';
    }
    // $fizz_numberのみの倍数
    elseif (($i % $fizz_number) === 0) {
        echo 'Fizz';
    }
    // $buzz_numberのみの倍数
    elseif (($i % $buzz_number) === 0) {
        echo 'Buzz';
    }
    // $fizz_numberもしくは$buzz_numberの倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

修正5：仕様変更に耐えうる記述をする2

修正2では、Fizz,Buzzを表示する数値に仕様変更があった場合に対応するための修正でした。
今回は、そのFizz,Buzzに仕様変更があり、他の文字列を表示する必要が出てきた場合を考えてみます。

現在の状態だと、'FizzBuzz'と、'Fizz'と、'Buzz'の３箇所を修正しなければなりません。
では、どうすれば修正箇所を減らすことができるでしょうか。

修正2の＄fizz_numberと＄buzz_numberの要領でいくと、変数に格納してしまえば一見良さそうです。

<?php

$fizz_number = 3;
$buzz_number = 5;
$fizz        = 'Fizz';
$buzz        = 'buzz';

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $fizz_numberかつ$buzz_numberの倍数
    if ((($i % $fizz_number) === 0) && (($i % $buzz_number) === 0)) {
        echo $fizz . $buzz;
    }
    // $fizz_numのみの倍数
    elseif (($i % $fizz_number) === 0) {
        echo $fizz;
    }
    // $buzz_numのみの倍数
    elseif (($i % $buzz_number) === 0) {
        echo $buzz;
    }
    // $fizz_numberもしくは$buzz_numberの倍数以外の数値
    else echo $i;

    echo PHP_EOL;
}

これで、とりあえずは文字列の'Fizz'と'Buzz'の2箇所を修正するだけで済みました。

しかし、echoの表示のされ方を意識できると、修正箇所を2箇所に抑えつつ、もう少しスッキリしたプログラムを書くことができます。

echoを使って連続で出力した時、改行されないことはご存知だと思います。
改行されないので、最後の行でPHP_EOLを使用しているわけですが、この改行されない仕組みを利用して、プログラムを書き換えてみます。

すると、以下のようになります。

<?php

$fizz_number = 3;
$buzz_number = 5;

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $fizz_numberの倍数であれば'Fizz'を出力
    if (($i % $fizz_number) === 0) {
        echo 'Fizz';
    }
    // $buzz_numの倍数であれば'Buzz'を出力
    if (($i % $buzz_number) === 0) {
        echo 'Buzz';
    }
    // $fizz_numberの倍数でも$buzz_numberの倍数でもない場合は$iを出力
    if (($i % $fizz_number) !== 0 && ($i % $buzz_number) !== 0) {
        echo $i;
    }

    echo PHP_EOL;
}

このようにすると、2つの数字の公倍数の時は両方のif文を通過し、その間に改行が無いので、'Fizz''Buzz'つまり、'FizzBuzz'と表示されます。
この書き方でも、修正するのは文字列の'Fizz'と'Buzz'の2箇所で済みます。

修正5：共通化する

さて、プログラムがだんだんと綺麗になってきましたが、修正4のプログラムをよく見てみてください。
同じような記述が複数回出てきているのに気が付きましたでしょうか？

そう、
($i % $fizz_number) === 0と、($i % $fizz_number) !== 0
($i % $buzz_number) === 0と、($i % $buzz_number) !== 0
の4つです。

イコールとノットイコールで違う式のように見えますが、!(($i % $fizz_number) === 0)とすれば、全く同じ記述の部分が2つずつ出来上がります。
これは、工夫次第でよりスッキリしたプログラムに仕上げることができます。

if文の中身は、式です。
分かりやすく書くと、if (式)です。
$piyo = $hoge + $fugaというプログラムにおける、$hoge + $fugaも式です。
となると、$piyo = 式です。
すると、$piyo = 100というプログラムの100も式と言えます。
なので、if (100)という風に書くことも多くの言語で可能です。

ここで、先程のif文を見てみます。例として、一番上のif文を抜粋して見てみましょう。
if (($i % $fizz_number) === 0)
というものですが、この($i % $fizz_number) === 0は上記で示したように、式です。
そして、同じく上記で示したように、$piyo = 式です。
ということは、以下のような書き方ができます。

$is_fizz = ($i % $fizz_number) === 0

＄is_fizzの中身には、真理値が入ります。
ことのことを踏まえてプログラムを書き換えると、以下のようになります。

<?php

$fizz_number = 3;
$buzz_number = 5;

// 1 ~ 100を出力
for ($i = 1; $i <= 100; $i++) {
    // $fizz_number, $buzz_numberで割り切れるかどうかを変数に入れておく
    $is_fizz = ($i % $fizz_number) === 0;
    $is_buzz = ($i % $buzz_number) === 0;
    
    // $fizz_numberで割り切れる
    if ($is_fizz) {
        echo 'Fizz';
    }
    
    // $buzz_numberで割り切れる
    if ($is_buzz) {
        echo 'Buzz';
    }
   
    // $fizz_numberでも$buzz_numberでも割り切れない 
    if (!$is_fizz && !$is_buzz) {
        echo $i;
    }
    
    echo PHP_EOL;
}

かなりスッキリしたと思います。
繰り返し記述されていたif文の条件に当たる式を、先に変数に真理値として格納しておき、後はその変数を使用していくことで、長ったらしい条件式を何度も記述しなくて済みました。

修正7:コメントは書かない

上記のプログラムには、読み手にどんなプログラムなのかというのが伝わりやすいようにコメントが添えてあります。親切ですね。
ではなぜこのコメントが不要なのか？
それは、「何も情報が増えていないから」です。
コメントは、あくまでも「なぜそのようなプログラムになっているのか？」という、プログラムだけでは読み手に伝わらないことを書く、というのが基本です。

今回の場合、// 1 ~ 100を出力などのコメントの中身は、プログラムを見ればわかります。ここにはプログラムに書かれていること以上の情報はありません。
また、コメントを書くということは、プログラムの修正にあたってコメントも管理・修正をしないといけないということになります。
例えば、最初の方のコードには// 3もしくは5の倍数以外の数値のようなコメントがありました。ここで、3や5に仕様変更があった場合、プログラムだけでなくこのコメントにも修正を加えなければなりません。もしこのコメントの修正をし忘れると、コメントとプログラムの内容に差異が出てきてしまい、読み手を無駄に混乱させてしまいます。
そのため、基本的に意味のないコメントは書いてはなりません。

最後に

いかがだったでしょうか。
もちろん、これが100%正しい書き方でないこともあるかと思います。
少しでも違うな、と思ったり、うちの会社ではこういうメリットがあるからこういうルールにしている、など、ご意見ありましたら是非コメントください。