Automated Java fighting and battling simulation

Question

This is my second post since I've started coding/joined this site so be ready for some bad code and beginner questions!

I am just playing around with Java right now, as I believe this is a fun way to learn the language, so I decided to make a simple fight simulator. The simulator is meant to have each player start with a set amount of health (hitPoints) and they take turns hitting each other. Their damage dealt determined by a random number roll.

I just started trying to use multiple classes to make my code easier to ready and more. Correct I guess.

I have a question but also hope someone can give me pointers on how to improve my code in some ways. My questions is. I want to make it so when a player hits 0 health the program goes to the outCome() method but it currently allows 1 more hit after a player loses. So if the person who attacks first dies, they get 1 more hit in before the program ends but this is wrong and I'm not sure how to fix this. Here is my code.

This is my main class that runs everything.

package com.company;
import java.util.Scanner;

public class Battle {

    public static Scanner scan = new Scanner(System.in);

    public static void main(String[] args) {
        System.out.println("Please enter your username.");
        Players.userName = scan.nextLine();

        System.out.println(Players.userName + ", get ready for battle..");
        System.out.println("Press enter when ready.");
        scan.nextLine();

        while(Players.pHP > 0 && Players.oHP > 0) {
            Game.fight();
        }

        Game.outCome();
    }

}

This is my Player class which I believe is very unnecessary right now and would like some feedback on how to make it more useful than it is. This class feels entirely wrong to me.

package com.company;

public class Players {

    static String userName;
    static int hitPoints = 99;
    static int oHP = opponentHP();
    static int pHP = playerHP();

    public static int playerHP() {
        return pHP = hitPoints;
    }

    public static int opponentHP() {
        return oHP = hitPoints;
    }

}

And finally my Game class which kind of has the game code in it. This class looks messy to me and I believe I will improve on my organization with practice.

package com.company;

import java.util.Random;

public class Game {

    public static Random r = new Random();
    private static boolean isTurn = true;

    public static void fight() {
        int damage = r.nextInt(20);

        try {
            Thread.sleep(1000);
        } catch(InterruptedException ex) {
            Thread.currentThread().interrupt();
            System.out.println("INTERUPTED DELAY");
        }

        if(isTurn) {
            Players.oHP -= damage;
            if(Players.oHP < 0) {
                Players.oHP = 0;
            }
            System.out.println("You hit a(n) " + damage + " and now your 
    opponent has " + Players.oHP + " HitPoints.");
            isTurn = false;
        } else {
            Players.pHP -= damage;
            if(Players.pHP < 0) {
                Players.pHP = 0;
            }
            System.out.println("Your opponent hit a(n) " + damage + " and now 
    you have " + Players.pHP + " HitPoints.");
            isTurn = true;
        }
    }

    public static void outCome() {

        if (Players.oHP == 0 || Players.oHP < 0) {
            victory();
        } else {
            defeat();
        }
    }

    private static void victory() {
        System.out.println("You are the ultimate warrior.");
        System.out.println("Thanks for fighting..........");
    }

    private static void defeat() {
        System.out.println("You have been slain.");
        System.out.println("Thanks for fighting..........");
    }

}

Answer 1

Hi thanks for sharing your code!

Players class

Creating a class to represent a Player is a great idea, however your current class represents both players. If you want to have 2 players in your game, it might make more sense to have a Player class rather than a Players class.

Also try and expose the bare minimum that is possible by making as much private as possible. Don't reveal implementation details, only expose what is absolutely required.

Instantiating your players could look like this.

int hp = 100;
Player playerOne = new Player(playerName, hp);
Player playerTwo = new Player("Opponent", hp);

Where possible avoid using classes as a way of storing global / static variables. The benefit of creating classes is to provide a method of abstraction.

On top of this Player class your Game class is essentially just some static methods that you call from the main method. What if your code could look like this

Game game = new Game(player1, player2);
game.fight();

I re-wrote a version of your program, hopefully you find this useful.

Main

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);

        System.out.println("Please enter your username.");
        String userName = scan.nextLine();

        System.out.println("Please enter initial hp.");
        int hp = scan.nextInt();


        Player human = new Player(userName, hp);
        Player opponent = new Player("Opponent", hp);

        Game game = new Game(human, opponent);
        game.fight();
    }
}

Player

import java.util.Random;

public class Player {
    private final String userName;
    private int hp;


    public Player(String userName, int initialHp) {
        this.userName = userName;
        hp = initialHp;
    }

    public void attack(Player enemy) {
        Random rnd = new Random();
        int damage = rnd.nextInt(21) + 1;
        enemy.dealDamage(damage);
        System.out.println(getUserName() + " dealt " + damage + " to " + enemy.getUserName());
        System.out.println(enemy.getUserName() + " has " + enemy.getHp() + " hit points left!");
    }

    public void dealDamage(int damage) {
        hp -= damage;
        hp = hp < 0 ? 0 : hp;
    }

    public boolean isDead() {
        return hp <= 0;
    }

    public String getUserName() {
        return userName;
    }

    public int getHp() {
        return hp;
    }
}

Game

import java.util.Random;

public class Game {

    private final Player humanPlayer;
    private final Player aiPlayer;

    public Game(Player humanPlayer, Player aiPlayer) {
        this.humanPlayer = humanPlayer;
        this.aiPlayer = aiPlayer;
    }

    public void fight() {
        try {
            Thread.sleep(1000);
        } catch (InterruptedException ex) {
            Thread.currentThread().interrupt();
            System.out.println("INTERUPTED DELAY");
        }

        while (true) {
            humanPlayer.attack(aiPlayer);
            if (aiPlayer.isDead()) {
                System.out.println("Opponent is dead!");
                victory();
                return;
            }

            aiPlayer.attack(humanPlayer);
            if (aiPlayer.isDead()) {
                System.out.println("You are dead!");
                defeat();
                return;
            }
        }
    }

    private void victory() {
        System.out.println("You are the ultimate warrior.");
        System.out.println("Thanks for fighting..........");
    }

    private void defeat() {
        System.out.println("You have been slain.");
        System.out.println("Thanks for fighting..........");
    }

}

Some changes of note and general advice

Added some logic to the Player class where I thought it would be reasonable to do so. These should hopefully make the code more readable too.

The return statement in the method when either player dies will exit immediately.

Made it so each player has a different random number. Otherwise here the Player would win every time since they hit first.

I would recommend making values final if possible. E.g. the userName or the Random object.

Don't blindly make getters and setters for everything (this is a commonly encouraged practice especially when you're first learning to code), make getters only if you need to and setters if you really need to mutate the value.

Don't make member variables public. This makes it very hard to change your code. Use getters if you need to access the values.

small thing here, the line if (Players.oHP == 0 || Players.oHP < 0) could simply be replaced with if(Players.oHP <= 0)

A class should be considered a blue print for making an object, not a way of accessing all information about all the objects.

Hopefully this review was helpful for you, keep it up!