はじめに
気分転換にとあるスキル評価サイトに登録してみたところ、最初の問題が、FizzBuzzでした。
ただ単にFizzBuzzを解くのは面白くないので、自分なりに保守しやすいFizzBuzzはこんな感じだろうというのを書いてみます。
最終的に採用したコード
実は提出した解答とちょっと違うのですが、解答の提出後にリファクタリングした結果、以下のようになりました。
public final class FizzBuzz {
/**
* 1〜100までの数値を以下のように、標準出力に出力する。
* <p>
* <ul>
* <li>3の倍数かつ5の倍数の場合: "FizzBuzz"と出力</li>
* <li>3の倍数で、5の倍数ではない場合: "Fizz"と出力</li>
* <li>3の倍数ではなく、5の倍数の場合: "Buzz"と出力</li>
* <li>いずれでもない場合: 数値をそのまま出力</li>
* </ul>
*
* @param args 未使用
*/
public static void main(String[] args) {
for (int i = 1; i <= 100; i++) {
boolean fizz = (i % 3 == 0);
boolean buzz = (i % 5 == 0);
if (fizz && buzz) {
System.out.println("FizzBuzz");
} else if (fizz) {
System.out.println("Fizz");
} else if (buzz) {
System.out.println("Buzz");
} else {
System.out.println(i);
}
}
}
}
Javadocをきっちりと書く
main()
メソッドには、Javadocで仕様を記載しています。未使用の引数を残しておくことは通常はありえないのですが、今回は main()
メソッドのため、未使用の引数には「未使用」と記載しています。
説明用変数を導入
if (i % 3 == 0)
と書いても問題ないのですが、2回同じ表現が出てくるのがちょっと気持ち悪いので、説明用変数を入れています。
変数名は「3の倍数」「5の倍数」を表すほうがいいかもしれませんが、いい名前が思いつかなかったのと、今回は「3の倍数 & 5の倍数なら"fizz" & "buzz"と出力」という仕様があるため、fizz
とbuzz
という変数名にしました。
演算子の優先順位に迷わないように、カッコを付ける
boolean fizz = (i % 3 == 0);
これは、結果的に以下のように書いても同じです。
boolean fizz = i % 3 == 0;
しかし、カッコがないと、コードを読む人を一瞬惑わせます。読む人を惑わせないために、カッコを付けるとよいです。
中括弧は省略しない
以下のように書くこともできますが、中括弧を省略すると、あとで処理が追加になったときにミスが発生する可能性が高くなるため、自分は省略していません。
まあ、ちゃんとインデントされていたら気づくので、古い習慣かもしれませんが。
public final class FizzBuzz {
public static void main(String[] args) {
for (int i = 1; i <= 100; i++) {
boolean fizz = (i % 3 == 0);
boolean buzz = (i % 5 == 0);
if (fizz && buzz) System.out.println("FizzBuzz");
else if (fizz) System.out.println("Fizz");
else if (buzz) System.out.println("Buzz");
else System.out.println(i);
}
}
}
i % 15 == 0
とは書かない
条件判定に if (i % 15 == 0)
と書いているコードをよく見かけます。
これは間違いではないですが、FizzBuzzの仕様はには1度も「15」という数字は出てきません。
もちろん3x5なのは明らかですが、少しでも惑わせないように、15という数字は出さないようにしました。
クラスにはfinalを付ける
staticメソッドしかないクラスに継承もへったくれもないのですが、
過去にfinalをつけなかったばかりに勝手に継承されて泣いたことがあるので、
Exception系など一部を除いて、基本的にfinalを付けるようにしています。
Rangeは使わない
Javaでは範囲を示す方法が言語でサポートされていないため、以下のような書き方は採用せず、誰でも分かるシンプルなfor文を使いました。実際どれを使うかは状況次第ですが。
import java.util.stream.IntStream;
public static void main(String[] args) {
IntStream.rangeClosed(1, 100).mapToObj((i) -> {
if (i % 3 == 0 && i % 5 == 0) {
return "FizzBuzz";
} else if (i % 3 == 0) {
return "Fizz";
} else if (i % 5 == 0) {
return "Buzz";
} else {
return String.valueOf(i);
}
}).forEach(System.out::println);
}
もちろん言語でサポートされているRubyなら、以下のように書きます。
(1..100).each do |i|
...
end
不必要な抽象化をしない
例えば、「後々標準出力以外にも出力するかもしれないからListで返せるようにしたい」と考えて、以下のように書くこともできます(runというメソッド名はともかく)。
ただ、実際必要になることはほとんどなく、不必要に抽象化したために泣いたケースの方が多いです。必要になったときにリファクタリングするので十分です。
import java.util.ArrayList;
import java.util.List;
public final class FizzBuzz {
public static void main(String[] args) {
List<String> results = FizzBuzz.run();
for (String result : results) {
System.out.println(result);
}
}
private static List<String> run() {
List<String> results = new ArrayList<>();
for (int i = 1; i <= 100; i++) {
boolean fizz = (i % 3 == 0);
boolean buzz = (i % 5 == 0);
if (fizz && buzz) {
results.add("FizzBuzz");
} else if (fizz) {
results.add("Fizz");
} else if (buzz) {
results.add("Buzz");
} else {
results.add(String.valueOf(i));
}
}
return results;
}
}
おわりに
「FizzBuzzにまじになっちゃってどうするの。」という感じですが、何を考えてコーディングしているのかを書き残すのは結構面白かったです。