Java：if文が多すぎるコードを改善したい

Question

Javaのコードの書き換えに苦戦しています。

以下のコードだと、一応思うようには動作するのですが、if文が多すぎるコードになってしまって綺麗ではないな..と思っています。

もう少し綺麗な式に直す方法があれば教えていただけませんか？

やりたいこと：
少ない文字で同時にいくつかの値を返すゲームを作りたい。
2人のプレイヤーが4つのアクションボタンをそれぞれ持っており、結果は(0-3)の配列から算出する。
メソッド引数'one'と'two'で受け取った数値から勝敗を判断し、戻り値として 0=両者の負け、1=プレイヤー1の勝ち、2=プレイヤー2の勝ち、3=両者の勝ち、のどれかを返すというものです。

public int fightMath(int one, int two) {

 if(one == 0 && two == 0) { result = 0; }
 else if(one == 0 && two == 1) { result = 0; }
 else if(one == 0 && two == 2) { result = 1; }
 else if(one == 0 && two == 3) { result = 2; }
 else if(one == 1 && two == 0) { result = 0; }
 else if(one == 1 && two == 1) { result = 0; }
 else if(one == 1 && two == 2) { result = 2; }
 else if(one == 1 && two == 3) { result = 1; }
 else if(one == 2 && two == 0) { result = 2; }
 else if(one == 2 && two == 1) { result = 1; }
 else if(one == 2 && two == 2) { result = 3; }
 else if(one == 2 && two == 3) { result = 3; }
 else if(one == 3 && two == 0) { result = 1; }
 else if(one == 3 && two == 1) { result = 2; }
 else if(one == 3 && two == 2) { result = 3; }
 else if(one == 3 && two == 3) { result = 3; }

 return result;
}

Answer 1

計算量(比較演算を含む) は、質問文中の方法では、
最悪 16 回の比較、平均で 8 回
です。

最初の回答にあるように勝敗決定表を用意して、その表を引くことで、計算量を常に一定にできます。

しかし
もっとケース分けが膨大な量 (100 * 100) になる、
決定表に未使用のセルが大量に発生する、
ような場合は、勝敗決定を　ある程度 プログラムロジックで書く必要がでてきます。

質問のゲームの場合だと、 プレーヤーの手は何らかの物理的な操作に結びついて
いるとおもわれるので、その物理的な挙動にそったロジックを書けばよいです。
もしかすると、計算式１つで、勝敗の計算ができることもあるかもしれません。
(ここでの質問では、ゲームの手の物理的なモデルが不明なので、その例を私が示すことはできませんが)

そこで、以下に、なるべく 計算量がすくなくなるように少し工夫した例をしめします。
質問文では、プレーヤの手は　[1,2,3,4] ですが、それを [-2, -1, 1, 2] に変更しています。
そうすると、このゲームでは 勝敗結果が 0, 3 の場合のチェックが簡単にできるからです。
もっとうまい値や判定方法をみつければ、勝敗結果が 1, 2 の場合についても もっと単純にできそうな気がしますが、今はここまでで...

Sample.java

public class Sample {
    public static void main(String[] args) {
        int[] hands = {-2, -1, 1, 2};

        for( int one : hands) {
            for( int two : hands) {
                System.out.printf("[%d, %d] => %d\n", one, two, fightMath(one, two));
            }
            System.out.printf("¥n");
        }
    }

    // val　の符号を調べる。
    // @retrun -1: 負の値, 1: 正の値
    static int sign(int val) {
        return val < 0 ? -1 : 1;
    }

    // @param x  player one の手 (-2, -1, 1, 2)
    // @param y  player two の手 (-2, -1, 1, 2)
    // @return:  (ビット２： y の勝ち負け、ビット１: x の勝ち負け) とした整数を返す。
    static int fightMath(int x, int  y) {
        assert(x != 0);
        assert(y != 0);
        int win_x = 0;  // 0: 負け, 1: 勝ち
        int win_y = 0;  // 0: 負け, 1: 勝ち
        int val = x * y;

        if (val > 0) {
            win_x = win_y = (sign(val) == sign(x)) ? 1 : 0;
        } else if (val == -2) {
            win_x = (x < 0) ? 1 : 0;
            win_y = 1 - win_x;
        } else {
            win_x = (x > 0) ? 1 : 0;
            win_y = 1 - win_x;
        }
        return win_x + win_y * 2;
    }
}

このプログラムを走らせると次の様な出力になります。

$ javac Sample.java 
$ java -ea Sample
[-2, -2] => 0
[-2, -1] => 0
[-2, 1] => 1
[-2, 2] => 2

[-1, -2] => 0
[-1, -1] => 0
[-1, 1] => 2
[-1, 2] => 1

[1, -2] => 2
[1, -1] => 1
[1, 1] => 3
[1, 2] => 3

[2, -2] => 1
[2, -1] => 2
[2, 1] => 3
[2, 2] => 3

Comments

[do_jyo]

回答ありがとうございました！試してみようと思います！

Answer 2

色々な方法があると思いますが、結果が限られたパターンの数値になることが前提であれば、以下のような固定の配列を変換テーブルのようなイメージで１つ作ってしまって、添え字に引数をセットして結果を取り出す形にすればコード量が減らせると思います。

final int[][] result = new int[][] {
 { 0, 0, 1, 2 },
 { 0, 0, 2, 1 },
 { 2, 1, 3, 3 },
 { 1, 2, 3, 3 }
};
return result[one][two];

Answer 3

入力と出力の関係がコードを見てもよくわからなかったので、Excelで表を作ると、

• 0を返す時：両者とも0か1
• 1を返す時：{0,2},{1,3},{2,1},{3,0}
• 2を返す時：{0,3},{1,2},{2,0},{3,1}
• 3を返す時：両者とも2か3

となりました。これから次のようにできると思います（これが決してベストな方法だとは思っていませんが・・・）。

public int fightMath(int one, int two) {
 if((one == 0 || one == 1) && (two == 0 || two == 1)) return 0;
 else if((one == 2 || one == 3) && (two == 2 || two == 3)) return 3;
 else return evaluate(one, two);
}

public int evaluate(int one, int two) {
 switch(one) {
  case 0:
   if(two == 2) return 1;
   else if(two == 3) return 2;
   break;
  case 1:
   if(two == 2) return 2;
   else if(two == 3) return 1;
   break;
  case 2:
   if(two == 0) return 2;
   else if(two == 1) return 1;
   break;
  case 3:
   if(two == 0) return 1;
   else if(two == 1) return 2;
   break;
 }
}