javaで作成したプログラムコードを短くしたい

Question

先日は無礼な質問の投稿を失礼いたしました。

自分なりに改善を重ねた結果分からない点が出ましたので質問致します。

javaで作成したジャンケンプログラムのコードを短くしたいと考えています。

※作っているゲームの仕様や機能は以下の通り
■キーボードから入力して敵と主人公に名前を付ける
■敵と主人公でじゃんけんをする、(主人公の手はコマンド入力、敵の手は自動)
■じゃんけんで勝った方が負けた方にダメージを与える(主人公と敵のHPは20)
■ダメージは１～３のランダム
■10分の１の確立でクリティカルヒット、上記のダメージの５倍のダメージ
(１ダメージがクリティカルだと５ダメージになる)
■主人公のHPが0になったらyou-Rosstと表示しゲーム終了
■敵のhpが0になるとyou-Winと表示してゲーム終了

短くしたいと考えているコード

「ジャンケンに主人公が勝利した場合のダメージ処理メソッド」と「ジャンケンに敵が勝利した場合のダメージ処理メソッド」があるクラスです。

自分なりに変数などのを使って戦闘時のクリティカル分岐のコードを短くしました。

ですがSystem.out.println();を使った同じような内容の文章を下記のコードで四回繰り返してしまっているのですが、更に短くすることはできるのでしょうか？

****主人公が勝利した場合のメソッドも敵が勝利した場合のメソッドも似たような処理なので、何かすれば更に短く出来そうなのですがその何かが分かりません。

---

ヒントをいただけますでしょうか?

public class BattleSystem{

    /*フィールド値*/

    public int playrHp = 20;//主人公のHP
    public int enemiHp = 20;//敵のHP




    /*主人公が勝利した場合の戦闘処理メソッド*/
    void ataltuku(){
         this.playrHp = playrHp;
         this.enemiHp=enemiHp;

        java.util.Random rand = new java.util.Random();
        java.util.Random rand2 = new java.util.Random();
        int damag = rand.nextInt(3)+1;//ダメージのランダム数字1～3を取得
        int criticalCheck = rand2.nextInt(10);//クリティカル判定　9の時にクリティカル
        int criticalDamage = damag * 5;//クリティカル判定時のダメージ【com*5倍のダメージ】

        /*戦闘時のダメージ分岐*/

            if(criticalCheck < 8  ) {
                System.out.println(Main.nameInfo.inputName+"の勝ち");
                System.out.println(Main.nameInfo.inputEnemi+"は"+damag+"のダメージを受けた");
                this.enemiHp= enemiHp-damag;
                System.out.println(Main.nameInfo.inputEnemi+"HP:"+this.enemiHp+"/20");
                System.out.println(Main.nameInfo.inputName+"HP:"+this.playrHp+"/20");
                System.out.println("-----------------------------------------");
            }
            if(criticalCheck == 9 ) {
                System.out.println(Main.nameInfo.inputName+"の勝ち");
                System.out.println(Main.nameInfo.inputEnemi+"は"+criticalDamage+"のダメージを受けた");
                this.enemiHp= enemiHp-criticalDamage;
                System.out.println(Main.nameInfo.inputEnemi+"HP:"+this.enemiHp+"/20");
                System.out.println(Main.nameInfo.inputName+"HP:"+this.playrHp+"/20");
                System.out.println("-----------------------------------------");
            }


    }//ataltuku()

    /*敵が処理した場合の戦闘処理メソッド*/
    void dameizi(){
        this.playrHp = playrHp;
        this.enemiHp=enemiHp;
        java.util.Random rand = new java.util.Random();
        java.util.Random rand2 = new java.util.Random();
        int damag = rand.nextInt(3)+1;//ダメージのランダム数字1～3を取得
        int criticalCheck = rand2.nextInt(10);//クリティカルのランダムな数値 9でクリティカル
        int cd = damag * 5;//クリティカル判定時のダメージ【com*5倍のダメージ】


            if( criticalCheck < 8 ) {
            	
                System.out.println(Main.nameInfo.inputEnemi+"の勝ち");
                System.out.println(Main.nameInfo.inputName+"は"+damag+"のダメージを受けた");
                this.playrHp=playrHp-damag;
                System.out.println(Main.nameInfo.inputName+"HP:"+this.playrHp+"/20");
                System.out.println(Main.nameInfo.inputEnemi+"HP:"+this.enemiHp+"/20");
                System.out.println("-----------------------------------------");
            }
            if( criticalCheck == 9 ) {
                System.out.println(Main.nameInfo.inputEnemi+"の勝ち");
                System.out.println(Main.nameInfo.inputName+"は"+cd+"のダメージを受けた");
                this.playrHp=playrHp-cd;
                System.out.println(Main.nameInfo.inputName+"HP:"+this.playrHp+"/20");
                System.out.println(Main.nameInfo.inputEnemi+"HP:"+this.enemiHp+"/20");
                System.out.println("-----------------------------------------");
                }


    }//dameizi()

}

Answer 1

今のコードを最大限尊重すると、こんな感じでしょうか。

• 中途半端な命名見直し
  すいませんguとか使って^^;
• Main.nameInfoへの依存解消
  結局BattleSystemでしか使っていないうえ、長くて冗長。
• 変わらない変数はfinal
  意図せず変更されないように。
• 表示順をあなた敵に固定
  おそらくわざとやっているのでしょうが、共通化しにくいのと入れ替わると意外と見にくい。

import java.util.Random;
import java.util.Scanner;

public class Main {
    public static void main(String[] args) {
        final Scanner scanner = new Scanner(System.in);
        final Random rand = new Random();

        // NamedはBattleSystemでしか使ってないのであげちゃう ついでにRandomも
        final BattleSystem status = new BattleSystem(new Named(scanner), rand);

        // 数字だとわかりにくいので
        final int gu = 0;
        final int tyoki = 1;
        final int pa = 2;

        while (0 < status.playerHp && 0 < status.enemyHp) {
            System.out.println("-----------------------------------------");
            System.out.println("コマンド? 0:グー　1:チョキ　2:パー");
            int hand = scanner.nextInt();

            if (hand == 9) {
                System.out.println("コマンド９検出：強制終了しました");
                return; // 勝ち負け判定に入らないように抜ける
            }

            int com = rand.nextInt(3);

            if (hand == com) {
                System.out.println("引き分け");
            }
            // かっこや改行があれば長い条件も結構わかりやすくないですか
            else if ((com == gu && hand == pa) ||
                    (com == tyoki && hand == gu) ||
                    (com == pa && hand == tyoki)) {
                status.attack();
            } else {
                status.damage();
            }
        }

        System.out.println("-----------------------------------------");
        if (status.enemyHp <= 0) {
            System.out.println("☆YOU WIN☆");
        } else {
            System.out.println("✖YOU LOSE✖");
        }
    }
}

class Named {
    final String playerName;
    final String enemyName;

    // どうせinput_youname するならコンストラクタでいいでしょう メンバ変数もfinalにできる
    Named(Scanner scanner) {
        System.out.print("あなたの名前を入力してください ▼ ");
        playerName = scanner.nextLine();
        System.out.println(playerName + "が入力されました");

        System.out.print("敵の名前を入力してください ▼ ");
        enemyName = scanner.nextLine();
        System.out.println(enemyName + "が入力されました");

        System.out.println();
        System.out.println("あなたの名前は" + playerName + "です");
        System.out.println("敵の名前は" + enemyName + "です");
    }
}

class BattleSystem {
    private final Named nameInfo;
    private final Random rand;

    int playerHp = 20;
    int enemyHp = 20;

    BattleSystem(Named named, Random random) {
        nameInfo = named;
        rand = random;
    }

    void attack() {
        int damag = getDamagePoint();
        enemyHp -= damag; // enemiHp = enemiHp - damag; 同じ意味
        print(nameInfo.playerName, nameInfo.enemyName, damag);
    }

    void damage() {
        int damag = getDamagePoint();
        playerHp -= damag;
        print(nameInfo.enemyName, nameInfo.playerName, damag);
    }

    private int getDamagePoint() {
        int damag = rand.nextInt(3) + 1;
        if (rand.nextInt(10) == 9) { // クリティカル時 5倍
            damag *= 5; // damag = damag * 5; 同じ意味
        }
        return damag;
    }

    private void print(String winner, String loser, int damage) {
        System.out.println(winner + "の勝ち");
        System.out.println(loser + "は" + damage + "のダメージを受けた");

        System.out.println();
        System.out.println(nameInfo.playerName + " HP:" + playerHp + "/20");
        System.out.println(nameInfo.enemyName + " HP:" + enemyHp + "/20");
    }
}

---

ゲームの修正点１　継承・抽象メソッドを使用したゲームにする
具体的にはCpuとHuman(人間)をそれぞれ別のクラスとして作り、共通の親クラスPlayerを継承するようにしたいです。

もともとの内容にもついでに一例を出しておきます（あんまりイケてないですｗ
９割方できているのでclass Human・class Cpuを作ってみてください^^（int getHand()を実装するだけです）

import java.util.Random;
import java.util.Scanner;

public class Main {
    public static void main(String[] args) {
        // 人間を表すHumanクラスからインスタンスを一つ生成します
        // ゲームに参加する人間は一人なので、new Humanするのはここでの1回限り
        // ほかでnew Humanしてしまうと別の人間が新たに参加することになってしまう！
        Player player = new Human("あなた", 20);

        // CPUを表すHumanクラスからインスタンスを一つ生成します
        // ゲームに参加するCPUは一個なので、...以下同文
        Player cpu = new Cpu("敵", 20);

        System.out.println();
        System.out.println("あなたの名前は" + player.getName() + "です");
        System.out.println("敵の名前は" + cpu.getName() + "です");

        final int gu = 0;
        final int tyoki = 1;
        final int pa = 2;

        // どちらとも生きている（0 < Hp）間はループ
        while (player.isAlive() && cpu.isAlive()) {
            int hand = player.getHand(); // playerの手を取得
            if (hand == 9) {
                System.out.println("コマンド９検出：強制終了しました");
                return; // 勝ち負け判定に入らないように抜ける
            } else if (2 < hand) {
                System.out.println("数値は0・１・２から選んで入力して下さい");
                continue; // ループ先頭に戻り手の再入力
            }

            int com = cpu.getHand(); // cpuの手を取得

            if (hand == com) {
                System.out.println("引き分け");
                continue;
            } else if ((com == gu && hand == pa) ||
                    (com == tyoki && hand == gu) ||
                    (com == pa && hand == tyoki)) {
                player.attack(cpu); // playerの勝ち playerがcpuに攻撃
            } else {
                cpu.attack(player); // cpuの勝ち cpuがplayerに攻撃
            }

            System.out.println();
            System.out.println(player); // toString()をオーバ-ライド 名前 HP:現在値/Max値
            System.out.println(cpu); // 同上
        }

        System.out.println("-----------------------------------------");
        if (player.isAlive()) {
            System.out.println("☆YOU WIN☆");
        } else {
            System.out.println("✖YOU LOSE✖");
        }
    }
}

abstract class Player {
    // あまりstaticは使いたくないが、まあprotected finalならいいでしょう^^;
    // Scannerに依存するのもどうなの？感ありｗ
    protected static final Scanner scanner = new Scanner(System.in);
    protected static final Random rand = new Random();

    private final String name;
    private final int maxHp;

    private int hp;


    Player(String tmpName, int hp) { // 呼びかけ用の仮名、初期hp
        this.hp = maxHp = hp;

        System.out.print(tmpName + "の名前を入力してください ▼ ");
        name = scanner.nextLine();
        System.out.println(name + "が入力されました");
    }

    String getName() { return name; } // 名前の取得

    boolean isAlive() { return 0 < hp; } // 生きているかどうか

    abstract int getHand(); // 手の番号取得 0:グー　1:チョキ　2:パー

    // 相手に攻撃
    // なぜ引数で相手を与えるかというと、両方のnameが欲しいから
    void attack(Player other) {
        int damage = rand.nextInt(3) + 1;
        if (rand.nextInt(10) == 9) {
            damage *= 5;
        }
        other.hp -= damage; // 相手のHpを減らす

        System.out.println(name + "の勝ち");
        System.out.println(other.name + "は" + damage + "のダメージを受けた");
    }

    @Override public String toString() { return name + " HP:" + hp + "/" + maxHp; }
}

class Human extends Player {
    Human(String tmpName, int hp) {
        super(tmpName, hp);
    }

    @Override int getHand() {
        System.out.println("-----------------------------------------");
        System.out.println("コマンド? 0:グー　1:チョキ　2:パー");
        return scanner.nextInt();
    }
}

class Cpu extends Player {
    Cpu(String tmpName, int hp) {
        super(tmpName, hp);
    }

    @Override int getHand() { return rand.nextInt(3); }
}

Answer 2

共通の親クラスPlayerに、CpuとPlayerで共通の変数（hp,name）や処理（ダメージをうける処理）などを定義し、それぞれ異なる処理を抽象メソッドを利用した形に変更するにはどうすればよいでしょうか？

敵味方に共通しているものは、どちらもゲームに登場するキャラクターである事です。
つまり、このキャラクターを親クラスとすれば良いでしょう。
貴方のクラスを尊重するとしたらこんな感じのクラスを親クラスとしておくのが良いでしょう。

public class Character{

    private String name;
    private int hp;

    Character(String char_name, int char_hp){
        hp = char_hp;
        name = char_name;
    }

    //戦闘処理メソッド(継承用)
    void battle(Character character){}
    //ダメージ用関数
    void damage(int sub){ hp -= sub; }
    //死亡判定
    bool isDead(){
        if (hp <= 0) return true;
        return false;
    }
    
    String getName(){ return name; }
}

同じような処理を繰り返すハードコーディングを避けた設計にするにはどうすれば良いでしょうか？

まず、2~3行以上同じだなと思う部分を関数化してしまいましょう。
後は一度関数化したからと言って放置してしまう事をせず、常に頭の中にソースコードをイメージしながら書いていきましょう。