c# / unity / unity2019.LTS / 魔改造 Judg○

[魔改造 Judg○] (6)問題自動生成部分を整理する

地味な話が続いてすみませんが、ご勘弁ください。

さて、問題自動生成部分をものすごく単純にかけば、こんな感じになります。
（row はディープコピー済みのバッファです）

switch (row.Question2)
{
    case "{TashizanX_X}":
        int a = Rand.Range(1, 10);
        int b = Rand.Range(1, 10);
        int c = Rand.Range(1, 5);
        if (Rand.Range(0, 2) == 1)
        {
            c *= -1;
        }

        row.Question2 = $"{a} + {b} =";
        row.Answer    = $"{a+b}";
        row.Miss[0]   = $"{a+b+c}";
        break;
    case "{TashizanXX_X}":
        int a = Rand.Range(10, 100);
        int b = Rand.Range(1, 10);
        int c = Rand.Range(1, 10);
        if (Rand.Range(0, 2) == 1)
        {
            c *= -1;
        }

        row.Question2 = $"{a} + {b} =";
        row.Answer    = $"{a+b}";
        row.Miss[0]   = $"{a+b+c}";
        break;
    case "{TashizanXX_XX}":
        int a = Rand.Range(10, 100);
        int b = Rand.Range(10, 20);
        int c = Rand.Range(1, 10);
        if (Rand.Range(0, 2) == 1)
        {
            c *= -1;
        }

        row.Question2 = $"{a} + {b} =";
        row.Answer    = $"{a+b}";
        row.Miss[0]   = $"{a+b+c}";
        break;
}

…。
まあ、動く。動くからこれでも。いや…。
問題が増えるほどに、とんでもない事になっていくのが想像できるでしょうか。

switch ～ case はサッと書けて便利ですが、ワンブレスで長文を読み上げるようなもの。
切れ目がハッキリせず目が滑り、結果修正に時間のかかる悪手になることも多いです。

でも初心者大好き命令。直感的にわかりやすいからでしょうか。

プログラムも「言語」であって、短く区切っていたほうが説明がつきやすいし、頭の中で整理しやすい。
「３行でまとめる」ことは出来ないかもしれませんが、わからないうちはとにかくメソッドを細かく、小さくしていくにはどうすればいいか、絶えず考えるといいと思います。

では、少しずつ、順を追って整理を進めていきましょう。

メソッド呼び出しにする

プログラムが短くなるわけではありませんが、switch ～ case で「何をしたいのか」がよりハッキリしたと思います。短く区切った効果です。

{
    switch (row.Question2)
    {
        case "{TashizanX_X}":
            createTashizanX_X(row);
            break;
        case "{TashizanXX_X}":
            createTashizanXX_X(row);
            break;
        case "{TashizanXX_XX}":
            createTashizanXX_XX(row);
            break;
    }
}

static void createTashizanX_X(Questions_Table.Row row)
{
    int a = Rand.Range(1, 10);
    int b = Rand.Range(1, 10);
    int c = Rand.Range(1, 5);
    if (Rand.Range(0, 2) == 1)
    {
        c *= -1;
    }

    row.Question2 = $"{a} + {b} =";
    row.Answer    = $"{a+b}";
    row.Miss[0]   = $"{a+b+c}";
}

static void createTashizanXX_X(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(1, 10);
    int c = Rand.Range(1, 10);
    if (Rand.Range(0, 2) == 1)
    {
        c *= -1;
    }

    row.Question2 = $"{a} + {b} =";
    row.Answer    = $"{a+b}";
    row.Miss[0]   = $"{a+b+c}";
}

static void createTashizanXX_XX(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(10, 20);
    int c = Rand.Range(1, 10);
    if (Rand.Range(0, 2) == 1)
    {
        c *= -1;
    }

    row.Question2 = $"{a} + {b} =";
    row.Answer    = $"{a+b}";
    row.Miss[0]   = $"{a+b+c}";
}

Dictionary を使う

switch ～ case の部分が１行になりました。createTashizan～ は前と同じです。
（厳密にはエラーチェックコードなども入れたいところですが）

C# も進化しているのでわかりませんが、Dictionary の検索速度は非常に速いので、数が多くなると switch よりもパフォーマンス高いかも？　なによりスッキリ書けるのがいいですね。

Action は C で言うところの「関数ポインタ」に近いものです。戻り値ありなら Func を使います。

static Dictionary<string, System.Action<Questions_Table.Row>>
    createMethods = new Dictionary<string, System.Action<Questions_Table.Row>>
{
    { "{TashizanX_X}", createTashizanX_X },
    { "{TashizanXX_X}", createTashizanXX_X },
    { "{TashizanXX_XX}", createTashizanXX_XX },
};

{
    createMethods[row.Question2](row);
}

createTashizan で共通化できる部分を外に出してみる

createTashizan の中を整理しました。
c は 0 を除いた -n ～ n を取得したかったので複数行の記述になっていましたが、メソッドにしてしまえば 1 行で済みます。
row に値を入れている場所も全部共通だったので、まとめてしまいました。

static void createTashizanX_X(Questions_Table.Row row)
{
    int a = Rand.Range(1, 10);
    int b = Rand.Range(1, 10);
    int c = getMinusPlusRangeWithoutZero(1, 5);

    setQAtext(row, a, b, c);
}

static void createTashizanXX_X(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(1, 10);
    int c = getMinusPlusRangeWithoutZero(1, 10);

    setQAtext(row, a, b, c);
}

static void createTashizanXX_XX(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(10, 20);
    int c = getMinusPlusRangeWithoutZero(1, 10);

    setQAtext(row, a, b, c);
}

// sub routine
static int getMinusPlusRangeWithoutZero(int min, int max)
{
    int c = Rand.Range(min, max);
    if (Rand.Range(0, 2) == 1)
    {
        c *= -1;
    }
    return c;
}

// sub routine
static void setQAtext(Questions_Table.Row row, int a, int b, int c)
{
    row.Question2 = $"{a} + {b} =";
    row.Answer    = $"{a+b}";
    row.Miss[0]   = $"{a+b+c}";
}

共通化の失敗例

共通化は「やりすぎない」事も重要です。
今は同じだけど、将来的には変わってしまいそうな部分など、共通化するとかえって足を引っ張ります。
例えば、無理やりですがこんなまとめ方も出来ちゃうはずです。

static void createTashizanX_X(Questions_Table.Row row)
{
    int a = Rand.Range(1, 10);
    int b = Rand.Range(1, 10);
    int c = getMinusPlusRangeWithoutZero(1, 5);
}

static void createTashizanXX_X(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(1, 10);
    int c = getMinusPlusRangeWithoutZero(1, 10);
}

static void createTashizanXX_XX(Questions_Table.Row row)
{
    int a = Rand.Range(10, 100);
    int b = Rand.Range(10, 20);
    int c = getMinusPlusRangeWithoutZero(1, 10);
}

↓

static void createTashizanX_X(Questions_Table.Row row)
{
  getABC(1, false, 1, out int a, out int b, out int c);
}

static void createTashizanXX_X(Questions_Table.Row row)
{
  getABC(10, false, 2, out int a, out int b, out int c);
}

static void createTashizanXX_XX(Questions_Table.Row row)
{
  getABC(10, true, 2, out int a, out int b, out int c);
}

static int getABC(int a_mul, bool b_from10, int c_mul, out int a, out int b, out int c)
{
    a = Rand.Range(1 * a_mul, 10 * a_mul);
    b = b_from10 == false ? Rand.Range(1, 10) : Rand.Range(10, 20);
    c = getMinusPlusRangeWithoutZero(1, 5 * c_mul);
}

3 行が全部 1 行にまとまった！　スッキリ！
わたしが初心者の頃であれば「俺って天才か…」と、悦にいることでしょう。
確かに、短くはなりました。

ですがこれは、部屋を片付けようとして「押し入れに全部突っ込んだ」状態です。
押し入れの中のペンケースを取り出さなければいけなくなった場合、果たしてどうなるでしょうか？

プログラムも同じ事で、こうなった後で

足し算 X+X だけ、1～10 じゃなくて3～7 にしよう

と言われたら「詰み」ですよね。bool a_from3to7 とか増えちゃうんでしょうか。

足し算の 3 桁対応もしよう！

無理です。許してください。
getABC() を使い続ける限り、不幸は雪だるま式に膨れ上がっていきます。

もうこのコードは触らせないでくれ。
自信に満ち、輝いていた彼（彼女？）も要求が増えるほど暗い影を落としていきますし、作った人が失踪してしまい、後を引き継ぐなんてことになったら目も当てられません。

驚いたことに、こういうとんでもプログラムは（きっちりと中抜きはした上で）外部から雇った社外プログラマに任せる会社が多いです。大手 SIer の悲劇はそうやって、今この時も産まれています。

プログラムをまとめるとき、「まとめた事で、それ以上変更できなくなる」ようなまとめ方を避けましょう。
高速化のため、どうしようもないケースももちろんありますが、ゲームロジックについては速度よりも汎用性を活かしておいた方が後々のためになると思います。

私は、といえば Dictionary のあたりまでやっておき、後はそのままにしています。
問題がどう変化するかわからないですし、数も少ないのでとりあえず createTashizan～ の中はそのままにしておこうかな、と。

「汎用性」も人によって粒度は様々。
「いざという時でも変更できるレベル」でまとめるにはどの程度がいいか、色々と作って、試してみてください。

基本的に、プログラマに変更をお願いすると嫌な顔をされるのは世界共通として（笑）、それでもすぐに変更してくれる人と、なかなか変更できなかったり、変更することですぐにバグを産み出してしまう人の差は、最初にどう上手くプログラムを整理していたか、でほぼ決まると思います。

言い換えると、他人が作ったぐっちゃぐちゃなプログラムは、いかな達人でも整理するのが難しい、ということです。
２年かけて作り上げたが動作が不安定なシステムを、「１から作ってよければ半年で作りますよ」というのは神業ではなく、そちらの方が本当に速くて楽、ということもあると思います。

related posts:

[魔改造 Judg○] (5)問題の作り方からディープコピーまで ゲームメニューの失敗と調整(14) Default Thumbnailヘルプ、難しいです…(17) レスポンスの悪さに気づく(9)