LoginSignup
@Soseki_A (冬耳)

Are you sure you want to delete the question?

If your question is resolved, you may close it.

Leaving a resolved question undeleted may help others!

We hope you find it useful!

じゃんけんゲームのレビューのお願い

Q&A

Java初心者コードレビュー

レビューのお願い

JavaでじゃんけんのCLIアプリケーションを作成しました。気づいた点があれば全て指摘いただけるとありがたいですが、特に以下の点においてレビューをいただきたいです。

  • 保守性・可読性(命名規則は一般的であるか。命名は分かりやすいものであるか。また、もっと分かりやすい例はあるか)
  • 拡張性(デスクトップやウェブで動くアプリケーションにしたり機能を追加したりする際、どれだけスムーズか。またはスムーズでないか)
  • 機能面(UIは十分であるか。また、不十分なものは何か)

アプリ仕様

ファイル数：2
インターフェース：CLI
参考元:【if編】Java言語で『じゃんけんゲーム』を作る

意識した点

オブジェクト指向に則ったクラスとメソッドの定義

コード

Janken.java

package Janken;
import java.util.Scanner;
public class Janken {
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        String[] hands = {"グー","チョキ","パー"};
        while (true) {
            System.out.println("じゃんけん…");
            System.out.println("グー:0, チョキ:1, パー:2");
            int playersHands = sc.nextInt();
            int computersHands = (int)(Math.random()*3);
            Judge judge = new Judge(playersHands, computersHands);
            String result = judge.result();
        if (playersHands < 0 || playersHands > 2) {
            System.out.println("不正な入力値です");
            continue;
        }
            System.out.println("プレイヤー:" + hands[playersHands]);
            System.out.println("コンピューター:" + hands[computersHands]);
            System.out.println("結果:" + result);
        }
    }
}

Judge.java

package Janken;
public class Judge {
    private int playersHands;
    private int computersHands;
    public Judge(int playersHands, int computersHands) {
        this.playersHands = playersHands;
        this.computersHands = computersHands;
    }
    public String result() {
        if (playersHands == computersHands) {
            return "あいこ";
        }
        switch (playersHands) {
            case 0:
                return (computersHands == 1) ? "勝ち" : "負け";
            case 1:
                return (computersHands == 2) ? "勝ち" : "負け";
            case 2:
                return (computersHands == 0) ? "勝ち" : "負け";
            default:
                return "不正な入力";
        }
    }
}
0 likes

1Answer

uasi
@uasi

保守性・可読性について

パッケージ名 package Judge はすべて小文字にするのが通例です。

int playersHands は1つの手を表しているため単数形にすべきです。また所有格の 's も省略して playerHand などとするのが一般的です。

Janken.java の if 文のインデントがずれています。

ユーザーが入力した hand 値の正しさを検証する責務がどこにあるか考えたうえで、 if 文の位置・内容を以下のように変えるべきです：

  • hand 値が0〜2の範囲に収まることを new Judge() の呼び出し側が保証すべきという設計にするなら、 if 文を new Judge() の前に移動する
  • 範囲に収まっているかどうかまで Judge クラスが判定する設計にするなら、 if 文は今の位置で result の値をチェックするように変える

拡張性について

この内容や規模感だと拡張性を持たせても無駄に読みづらくなるだけなので、考えない方がいいと思います。

ひとつ言うとすれば、 judge.result() の返り値は enum にしたほうがいいでしょう。勝ち負けの表現の変更や多言語化がしやすくなります。

機能面について

sc.nextInt() は入力が整数値としてパースできなければ例外をスローします。これを処理すべきです。

シンプルなじゃんけんゲームとしては機能面の不足はないと思います。

2Like

Comments

  1. @Soseki_A

    Questioner

    回答いただきありがとうございます。
    私はまだプログラミングに関する知識に乏しいため、変数名における所有格の省略など、細かい点までご指摘いただき大変助かります。

    ご指摘いただいた点を踏まえ、コードを修正してみました。修正前より洗練され、考慮漏れが減り、可読性も上がったと思います。また気になる点等あればコメントいただけると幸いです。
    自身が思う範囲ではenumを書く場所が妥当か判断つかず、不安です。

    修正コード

    Janken.java

    package janken;
import java.util.InputMismatchException;
import java.util.Scanner;
public class Janken {
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        String[] hands = {"グー","チョキ","パー"};
        int playerHand;
        while (true) {
            System.out.println("じゃんけん…");
            System.out.println("グー:0, チョキ:1, パー:2");
            try {
            	playerHand = sc.nextInt();
            } catch (InputMismatchException e) {
            	System.out.println("整数値で入力してください");
            	sc.nextLine();
            	continue;
            }
            int computerHand = (int)(Math.random()*3);
            Judge judge = new Judge(playerHand, computerHand);
            Result result = judge.result();
	        if (result == Result.INVALID) {
	            System.out.println(Result.INVALID);
	            continue;
	        }
            System.out.println("プレイヤー:" + hands[playerHand]);
            System.out.println("コンピューター:" + hands[computerHand]);
            System.out.println("結果:" + result);
    	    //if (result != Result.DRAW) break; じゃんけんを1回のみにする。 
        }
    }
}

    Judge.java

    package janken;
enum Result {
    WIN("勝ち"),
    LOSE("負け"),
    DRAW("あいこ"),
    INVALID("不正な入力");
    private final String label;
    Result(String label) {
        this.label = label;
    }
    @Override
    public String toString() {
        return label;
    }
}
public class Judge {
    private int playerHand;
    private int computerHand;
    public Judge(int playerHand, int computerHand) {
        this.playerHand = playerHand;
        this.computerHand = computerHand;
    }
    public Result result() {
        if (playerHand == computerHand) {
            return Result.DRAW;
        }
        switch (playerHand) {
            case 0:
                return (computerHand == 1) ? Result.WIN : Result.LOSE;
            case 1:
                return (computerHand == 2) ? Result.WIN : Result.LOSE;
            case 2:
                return (computerHand == 0) ? Result.WIN : Result.LOSE;
            default:
                return Result.INVALID;
        }
    }
}

Your answer might help someone💌

LoginSign Up