Zenn
Aidemy Tech BlogPublicationへの投稿
📚

FizzBuzz問題でソフトウェア設計の練習してみた 📚

nabe・2022/12/16に公開・1件のコメント
#
設計
tech

Aidemy Advent Calendar 2022 の参加記事です。

FizzBuzz問題といえばコーディングテストの問題としてお馴染みかと思います。今回はこのFizzBuzz問題を解くコードに対して実務的な観点を適用して設計の練習をしてみようと思います[1]。ソフトウェア設計についての考えや価値観みたいなものを緩く共有できればと思います🫠

FizzBuzz問題

この記事ではFizzBuzz問題を以下のように定義しようと思います。

1から100まで順に数を数えていき、数字をコンソールに出力して下さい。

ただし、数が

  • 3の倍数かつ5の倍数のときには "FizzBuzz"
  • 3の倍数のときには "Fizz"
  • 5の倍数のときには "Buzz"

を数字の代わりにコンソールに出力して下さい。

最初の解答

問題文に従ってコードを素直に書いてみると大体以下のような形になると思います[2]。言語はTypeScriptです[3]。このコードをこれから少しずつ書き換えていこうと思います。

function fizzbuzz(n: number) {
    for (let count = 0; count <= n; count++) {
        if (count % 15 === 0) {
            console.log({ count, message: 'FizzBuzz' });
        } else if (count % 3 === 0) {
            console.log({ count, message: 'Fizz' });
        } else if (count % 5 === 0) {
            console.log({ count, message: 'Buzz' });
        } else {
            console.log({ count, message: count });
        }
    }
}

fizzbuzz(100);

else不要

最初の回答を見ていてelse if 〜のところが少し読みづらいと思ったのでelseを消すことにします。処理結果が確定した時点でcontinueを使って次の反復処理に飛ぶようにします。

function fizzbuzz(n: number) {
    for (let count = 1; count <= n; count++) {
        if (count % 15 === 0) {
            console.log({ count, answer: 'FizzBuzz' });
            continue;
        }

        if (count % 3 === 0) {
            console.log({ count, answer: 'Fizz' });
            continue;
        }

        if (count % 5 === 0) {
            console.log({ count, answer: 'Buzz' });
            continue;
        }

        console.log({ count, answer: count });
    }
}

fizzbuzz(100);

行数は増えましたがif文の見た目が単純になりました。読みやすくなったんじゃないでしょうか。

ロジックを分離する

見た目が良くなったところで少し視点を変えてfizzbuzz関数の役割について考えてみようと思います。fizzbuzz関数は以下のように複数の役割を担っています。

  • 数を数え上げる
  • 解答を決定する
  • 解答をコンソールに出力する

経験上、複数の役割が1箇所に集中していると変更もそこに集中しがちです。解答を決定するロジックや解答の出力先が変わった場合fizzbuzz関数の実装を変更することになります[4]。

運用が長期化して変更が積み重なると役割が異なるコード同士が複雑に絡まり合い、スパゲティ🍝 や泥団子と揶揄されるような変更が難しい状態になります。このような状態に陥るとバグの頻発や開発速度の低下などの問題が起こるようになります。

上記のような状況を回避するにはfizzbuzz関数の役割を絞る必要があります。色々な方法があると思いますがここでは以下のように考えます。

  • FizzBuzz問題の定義で「数を数える」ことについて言及していたので数を数え上げる役割はfizzbuzz関数が持っていても不自然ではなさそう
  • 解答を決定するロジックはFizzBuzz問題の定義の一部なのでfizzbuzz関数から呼び出される形が直観的な気がする
  • 解答をコンソールに出力することはFizzBuzz問題について考える上で重要ではない。解答の出力方法はコードを実行している環境に依るはず

以上を踏まえてfizzbuzz関数には数を数え上げる役割だけ残し、それ以外の機能はfizzbuzz関数以外の場所に移すことにします。

type Answer = 'Fizz' | 'Buzz' | 'FizzBuzz' | number;

function getAnswer(count: number): Answer {
    if (count % 15 === 0) {
        return 'FizzBuzz';
    }

    if (count % 3 === 0) {
        return 'Fizz';
    }

    if (count % 5 === 0) {
        return 'Buzz';
    }

    return count;
}

function* fizzbuzz(n: number) {
    for (let i = 1; i <= n; i++) {
        yield { count: i, answer: getAnswer(i) };
    }
}

for (const answer of fizzbuzz(100)) {
    console.log(answer);
}

fizzbuzz関数には数を数える処理だけ残しました。そしてコンソールへの出力はfizzbuzz関数の呼び出し側に、解答を決定するロジックはfizzbuzz関数から呼び出すようにしました。

コード変更する場合の影響範囲が前よりも小さくなりました。

条件分岐を隠す

getAnswer関数にある条件分岐が少し冗長な気がしたのでMapに置き換えてみることにします。元のままでも十分にシンプルだと思うので実務だったらここまではやらないですが練習なので。

/**
 * 最小公倍数を求める
 */
function gcd(x: number, y: number): number {
    return x % y ? gcd(y, x % y) : y;
}

type Answer = 'Fizz' | 'Buzz' | 'FizzBuzz' | number;
const Answers: ReadonlyMap<number, (count: number) => Answer> = new Map<1 | 3 | 5 | 15, (count: number) => Answer>([
    [1, (count: number) => count],
    [3, () => 'Fizz'],
    [5, () => 'Buzz'],
    [15, () => 'FizzBuzz'],
]);

function getAnswer(count: number): Answer {
    // gcd(count:11, 15) => 1
    // gcd(count:6, 15) => 3
    // gcd(count:20, 15) => 5
    // gcd(count:15, 15) => 15
    const key = gcd(count, 15);

    /* 
      get(key)の結果がundefinedになることは基本的にあり得ないが
      型エラーを防ぐためにチェック(??演算子)を入れておく
    */
    return Answers.get(key)?.(count) ?? count;
}

function* fizzbuzz(n: number) {
    for (let i = 1; i <= n; i++) {
        yield { count: i, answer: getAnswer(i) };
    }
}

for (const answer of fizzbuzz(100)) {
    console.log(answer);
}

AnswersをgetAnswer関数のスコープに含めるか迷いましたが読み取り専用であるため関数の外に出しても問題になる可能性は低いと判断しました。getAnswer関数の中身を単純にすることを優先しました。

クラスにまとめてみる

解答に使うデータとロジックがまとまっていない点が少し気になったのでまとめてみることにします。1人でFizzBuzzをする場合を想像しながら以下のようにPlayerクラスにデータとロジックをまとめます。

function gcd(x: number, y: number): number {
    return x % y ? gcd(y, x % y) : y;
}

type Answer = 'Fizz' | 'Buzz' | 'FizzBuzz' | number;

class Player {
    private static readonly choices: ReadonlyMap<number, (count: number) => Answer> = new Map<1 | 3 | 5 | 15, (count: number) => Answer>([
        [1, (count: number) => count],
        [3, () => 'Fizz'],
        [5, () => 'Buzz'],
        [15, () => 'FizzBuzz'],
    ]);

    private count: number;

    constructor() {
        this.count = 0;
    }

    next(): { count: number; answer: Answer } {
        this.countUp();
        const answer = this.selectAnswer();
        return { count: this.count, answer };
    }

    private countUp(): void {
        this.count++;
    }

    private selectAnswer(): Answer {
        const key = gcd(this.count, 15);
        return Player.choices.get(key)?.(this.count) ?? this.count;
    }
}

function* fizzbuzz(n: number) {
    const p = new Player();
    for (const _ of [...Array(n)]) {
        yield p.next();
    }
}

for (const answer of fizzbuzz(100)) {
    console.log(answer);
}

書いておいてなんですがクラスにするメリットはあまり無さそうです。どこまで数えたかをPlayerが覚えておく必要は無いかもしれません。また、Playerはcount以外に状態を持ちません。クラスではなくモジュールとしてまとめた方が良いのかもしれません。

まとめ

読みやすさや変更のしやすさといった観点をFizzBuzz問題のコードに適用してみましたがいかがでしたでしょうか。参考になる部分ありましたら幸いです。

脚注

  1. ただし、投入する労力に見合う結果が得られるかどうかは気にしないです ↩︎

  2. デバッグのために数字を常にコンソールに出力するようにしてます ↩︎

  3. 仕事でよく使っているので ↩︎

  4. FizzBuzz問題の仕様が変わることは多分無いでしょうけど😅 ↩︎

Aidemy Tech Blog
Aidemy Tech BlogPublication

株式会社アイデミーの技術ブログです。

Discussion

Enterprise FizzBuzzを思い出す...

ログインするとコメントできます