Simple one-heap Nim game

Question

I wrote this game to practice some simple OOP. You play against the computer, but be aware that the computer has no real AI, as it just generates its moves randomly. The loser is the one whose turn it is when there is only one match left in the stack.

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

class Game {

    private boolean gameOver;
    Scanner input;
    Player computer, human;

    public Game() {

        computer = new Computer();
        human = new Human();
        input = new Scanner(System.in);

    }

    public void runGame(){


        System.out.println("Welcome to the game of Nim! How many matches would you like to play with?");
        human.setMatchesLeft(input.nextInt());



        while (!gameOver){

            if (computer.getMatchesLeft() != 1){
                System.out.print("Computers move: ");
                computer.playerTurn();
                computer.printStatus();


                if (computer.getMatchesLeft() == 1) {
                    gameOver = true;
                    System.out.println("Human loses, computer wins.");
                }
            }

            if (human.getMatchesLeft() != 1){
                System.out.print("Humans move: ");
                human.playerTurn();
                human.printStatus();

                if (human.getMatchesLeft() == 1) {
                    gameOver = true;
                    System.out.println("Computer loses, human wins.");
                }
            }

        }

    }

}

abstract class Player {


    private static int matchesLeft;

    public abstract void playerTurn();
    public abstract void printStatus();

    public void setMatchesLeft(int x){
        matchesLeft = x;
    }

    public int getMatchesLeft(){
        return matchesLeft;

    }

}

class Human extends Player {

    private int humanTurn;
    private Scanner input;

    public Human(){

        input = new Scanner(System.in);
    }


    public void playerTurn(){

        humanTurn = input.nextInt();

        while ( humanTurn > (getMatchesLeft() / 2) || humanTurn < 1 ) {
            System.out.print("Illegal move. Please pick at least 1 matches.\n");
            humanTurn = input.nextInt();
        }

        setMatchesLeft(getMatchesLeft() - humanTurn);

    }

    public void printStatus(){
        System.out.println("Human removes " + humanTurn + " matches. " + getMatchesLeft() + " matches left. ");
    }
}

class Computer extends Player {

    private int computerTurn;
    Random computerGen;

    public Computer(){
        computerGen = new Random();
    }


    public void playerTurn(){

            computerTurn = computerGen.nextInt(((getMatchesLeft() / 2) - 1) + 1) + 1;


        setMatchesLeft(getMatchesLeft() - computerTurn);

    }

    public void printStatus(){
        System.out.println(computerTurn);
        System.out.println("Computer removes " + computerTurn + " matches. " + getMatchesLeft() + " matches left. ");
    }


}

public class Nm {
    public static void main (String[] args){

        Game newgame = new Game();
        newgame.runGame();

    }
}

Answer 1

System.out.print("Illegal move. Please pick at least 1 matches.\n");

You've got a typo there, should be 1 match.

---

I like how you've structured your code, but there's a couple naming points I want to address...

When a player takes a turn, you call it a "player turn". Which sounds pretty okay... but then the code reads like this: human.playerTurn(); which doesn't sound all that great. "I am now doing my player turn". There is no verb in your function name. Functions do stuff, so they should have a verb in there, or else they might be not doing stuff. Or maybe they're doing vague stuff.

Try "player takes a turn". So you get this:

computer.takeTurn();

human.takeTurn();

Next up, you've got duplicated code.

        if (computer.getMatchesLeft() != 1){
            System.out.print("Computers move: ");
            computer.playerTurn();
            computer.printStatus();


            if (computer.getMatchesLeft() == 1) {
                gameOver = true;
                System.out.println("Human loses, computer wins.");
            }
        }

        if (human.getMatchesLeft() != 1){
            System.out.print("Humans move: ");
            human.playerTurn();
            human.printStatus();

            if (human.getMatchesLeft() == 1) {
                gameOver = true;
                System.out.println("Computer loses, human wins.");
            }
        }

This is pretty much the same thing twice. If you could make computer and human print "xxx's move: " then you'd be pretty close to merging the two sections of code into one...

Specifically, the parts that are different are...

if (computer.getMatchesLeft() != 1){
if (human.getMatchesLeft() != 1){

The human and computer in statements like this;

            System.out.print("Computers move: ");
            System.out.print("Humans move: ");

These two pieces of text;

                System.out.println("Human loses, computer wins.");
                System.out.println("Computer loses, human wins.");

And these two pieces of text. If you could remove those differences, or somehow extract them from the section of code to another place, you could merge the two sections of code. Having less duplicated code is good, because if you would have to make a change, you wouldn't have to make it in two places.

For this, I'd add a method to Player: getName(). That way, you could print player.getName()+"'s move: ". Implement in Computer as return "Computer"; and in Human as, well... your choice, maybe it just returns Human, maybe it returns a string the player has entered at the start of the game.

Then, you could use an array or a List to do this:

    Player[] players = {computer, human};
    Player victor = null;
    while (!gameOver){
        for(Player player : players){ //for each item in collection
            if(player.getMatchesLeft() != 1){
                System.out.print(player.getName()+"'s move: ");
                player.takeTurn();
                player.printStatus();
                if(player.getMatchesLeft() == 1){
                    victor = player;
                    gameOver = true;
                    break;
                }
            }
        }
   }

   if(victor == human){
      System.out.println("Computer loses, human wins."); 
   } else {
      System.out.println("Human loses, computer wins.");
   }

Which would simplify your game code quite a bit. You could make it even smaller by remembering that you don't need the matchesLeft check due to the existence of the break; statement.

I'd also take a look at moving the whole "Game" to a separate class. That way you could ask the player "do you want to play again" and start at the top. Aim for a structure like this:

  do {
      int matches = askForMatchCount();//maybe pass in Scanner, maybe use it from member variable
      Game game = new Game(human, computer);//Game(Player players...)
      game.setMatches(matches);
      Player victor = game.playGame();
      printVictoryMessage(victor);
 } while(askForRematch());

I've used some tricks there you might not have seen before. You'll have to google to find out what they are, but I'll help you with the search terms and short descriptions:

 do { /*code*/ } while( condition );

That's a do-while loop. It works like a while loop, but always runs at least once.

 Game(Player players...)

That ... in the argument list is called "varargs". It's useful if you have a number of arguments but don't know how many. In this case, I'm making it possible to give you a three player game.

---

Last point I have for now is misuse of static.

In Player, you have this:

 private static int matchesLeft;

And at first I was like "Why would you do this?" It makes no sense! The amount of matches left doesn't belong to 1 player, does it? Do they have their own matches? Isn't this a game where the matches are shared between the players... Oh.

You've made them shared between the players! Makes sense, I guess...

Except you've just made them shared between ALL the players. Which is not so good if you ever wanted there to be multiple games happening at the same time.

If you instead relocated the matches to the game (like I did a bit earlier with game.setMatches(int matches), then you can change the takeTurn to take a Game object and obtain the matches that way (takeTurn(Game game)).