Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Project contains 3 classes:

/*************************************************************************************
The purpose of this project is to create 3 classes. Pile contains the number of marbles
in a randomly generated pile, and also a remove method to determine how many marbles are
to be removed. The player class determines the behavior of each type of player. There are
three types of players. Simple computer, Smart computer, and Human. These are all of enum
type. The program will use a randomly generated number to determine who goes first and
which level of skill the computer player is set at. Each player will draw a number of marbles
from the pile until the pile reaches 1. Whoever pulls the last marble will lose.
A player must choose between 1 and half the remaining pile.
***************************************************************************************/

Game

import java.util.*;

public class Game {

    private static void printHeading(int projectNum, String projectName){ //creates an identifier method.

        System.out.println("Chris Olson");
        System.out.println("CMSC 255-002, Spring 2013");
        System.out.println("Project " + projectNum);
        System.out.println(projectName);
        System.out.println();
    }

    public static void main(String[] args) {

        printHeading(8,"Nim");
        Scanner in = new Scanner(System.in);

        Pile marbles = new Pile(); //creates initial Pile object


        //Sets variables and objects to enum type
        int marblesToRemove = 0;
        Player humanPlayer = new Player(Player.Type.HUMAN);
        Player compPlayer = new Player(Player.Type.SMART_COMPUTER);
        Player secondPlayer = new Player(Player.Type.HUMAN);
        Player firstPlayer = new Player(Player.Type.HUMAN);
        Random randomNum = new Random(); //creates new random object to generate random numbers
        int selection = 0;


        //Interface
        System.out.println("**************WELCOME TO THE GAME OF NIM******************");
        System.out.println();
        System.out.println();
        System.out.println("******************RULES OF THE GAME***********************");
        System.out.println("--The number of marbles in the initial pile will be random amount between 10 and 100.");
        System.out.println("---Each player must choose between 1 and half of the remaining pile of marbles to remove.");
        System.out.println("----The player stuck with the last marble loses the game."); 
        System.out.println("---The computer will randomly be placed into simple or smart mode randomly.");
        System.out.println("--The player who goes first will also be chosen at random.");
        System.out.println();

        System.out.println("Enter 1 to play against another human or 2 to play against a computer.");
        System.out.println("Enter 3 if you would like 2 computers to play against each other.");
        selection = in.nextInt();


        //Generates a random number between 1-2 to determine whether computer is set to smart of simple mode
        if(randomNum.nextInt(2) == 1){ 
            compPlayer.setType(Player.Type.SMART_COMPUTER);
            System.out.println("Computer is set to Smart mode.");
        }

        else{
            compPlayer.setType(Player.Type.SIMPLE_COMPUTER);
            System.out.println("Computer is set to Simple mode.");
        }

        if(selection == 1){
            compPlayer = humanPlayer;
        }
        else if(selection == 2) compPlayer = compPlayer;
        else if(selection == 3) humanPlayer = compPlayer;


        //Generates a random number between 1-2 to determine who goes first
        if(randomNum.nextInt(2) == 1){
            System.out.println("The user may go first.");
            firstPlayer = humanPlayer; //sets player order
            secondPlayer = compPlayer;
        }

        else{
            System.out.println("The computer will go first.");
            firstPlayer = compPlayer; //sets player order
            secondPlayer = humanPlayer;
        }

        System.out.println();
        System.out.println("Initial pile contains " + marbles.getMarbles() + " marbles.");
        System.out.println();

        while(marbles.getMarbles() > 0){ //while loop runs until getMarbles is greater than 0   

            System.out.println("Number of marbles currently in pile: " + marbles.getMarbles());
            System.out.println();
            marblesToRemove = firstPlayer.playTurn(marbles.getMarbles()); //calls playTurn and getMarbles methods to determine how many marbles have been removed and strategy

            for(int i = 0;i<10;i++){
                System.out.println("*");
            }

            System.out.println("--Player 1 removed " + marblesToRemove + " marbles.");
            marbles.removeMarbles(marblesToRemove); //calls removeMarbles method with argument marblesToRemove
            System.out.println();

            if(marbles.getMarbles() == 1){ //If player 2 ends up with 1 marble left, player 1 wins
                System.out.println("**********Player 1 wins.**********");
                break;
            }


            System.out.println("Number of marbles currently in pile: " + marbles.getMarbles());
            System.out.println();
            marblesToRemove = secondPlayer.playTurn(marbles.getMarbles()); //calls playTurn and getMarbles methods to determine how many marbles have been removed and strategy

            System.out.println("--Player 2 removed " + marblesToRemove + " marbles.");
            marbles.removeMarbles(marblesToRemove); //calls removeMarbles method with argument marblesToRemove
            System.out.println();

            if(marbles.getMarbles() == 1){ //If player 1 ends up with 1 marble left, player 2 wins
                System.out.println("**********Player 2 wins.**********");
                break;
            }
        }
    }
}

Player

import java.util.*;

/**
*  
* @param player class sets the player type to enum and then dictates behavior based on selection in Game class.
*/


public class Player {

    Scanner input = new Scanner(System.in);
    Random marbles = new Random(); //creates random object of random class

    public enum Type{HUMAN,SMART_COMPUTER,SIMPLE_COMPUTER} //declare enum objects

    private Type type;
    /**
    *  Constructor
    * @param t indicates which of the three allowable Types this Player object will be
    */ 
    public Player(Type t){
        this.type = t;
    }

    /**
        *  
    * @param t allows for enum type to be re-assigned
    */ 

    public void setType(Type t){
        this.type = t;
    }


    /**
    *  
    * @param playTurn determines the computer and human behavior that dictates the course of the game
    */ 


    public int playTurn(int pileSize){

        int marblesRemoved = 0; //sets variable to 0

        if(type == Type.SIMPLE_COMPUTER){ //SIMPLE_COMPUTER makes any random move, with no strategy.

            marblesRemoved = marbles.nextInt((pileSize)/2)+1; //removes a random number between 1 and half the size of the remaining pile
        }


        else if(type == Type.SMART_COMPUTER){ //SMART_COMPUTER plays with a strategy. the computer takes off enough marbles to make the size of the pile a power of two minus 1—that is, 3, 7, 15, 31, or 63

        if(pileSize > 63)
            marblesRemoved = pileSize - 63;

        else if(pileSize > 31)
            marblesRemoved = pileSize - 31;

        else if(pileSize > 15)
            marblesRemoved = pileSize - 15;

        else if (pileSize > 7)
            marblesRemoved = pileSize - 7;

        else if(pileSize == 2)
            marblesRemoved = pileSize - 1;

        else
            marblesRemoved = marbles.nextInt((pileSize)/2)+1; //if the size of the pile is between 3-7, SMART_COMPUTER makes random legal move


        }

        else if (type == Type.HUMAN){ //HUMAN chooses the amount of marbles to remove

            System.out.println("Please choose a number of marbles to remove between 1 and " + (pileSize/2));
            marblesRemoved = input.nextInt();

            while(marblesRemoved <=0 || marblesRemoved > pileSize/2){ //user input check to make sure number is between the correct values
                System.out.println("You must choose a number between 1 and " + (pileSize/2) + ". Please choose another number.");
                marblesRemoved = input.nextInt();
            }
        }

        return marblesRemoved; //return marbles removed based on player enum type
    }


    public String toString(){ //can be implicitly called and relevant information about the internal state of the object. shows which enum player type is being played.
        return this.type + "Player type selected.";
    }
}

Pile

import java.util.*;
/**
*  
* @param pile class generates a random pile of marbles to be used in game class. Ths number of marbles is between 10-100 inclusive.
 * It also calculates the number of marbles removed
*/ 

public class Pile {

    private int marbles = 0;
    Random rand = new Random(); //create new random object to generate random number


/**
    *  
    * @param pile constructer sets marble pile to random number between 1-100
    */ 
    public Pile(){

        this.marbles = rand.nextInt(89)+11;
    }

    /**
    *  
    * @param getMarbles returns the current number of marbles in the game
    */ 


    public int getMarbles(){
        return this.marbles;
    }

    /**
    *  
    * @param removeMarbles accepts an int vaiarble to remove from the stack of marbles
    */ 


    public void removeMarbles(int takeMarb){
        this.marbles -= takeMarb;
    }

    public String toString(){ //can be implicitly called and relevant information about the internal state of the object. Shows the current amount of marbles in stack.
        return this.marbles + "marbles left in pile.";
    }
}
share|improve this question

3 Answers 3

There is a lot of code in main(). It would be better to break this up into smaller pieces.


There is a lot of extra white space. There generally is no need for multiple consecutive empty lines.

Within playTurn(), you separate a sequence of if-else if blocks with an empty line. A better solution would be to use curly brackets. This will clearly mark the sections to both humans and computers. Without the curly brackets, adding a second indented statement after the first will always execute (or be a syntax error if it is followed by an else.


Enums are a bad way to implement different strategies. It requires a lot of boilerplate code to be written in many different places. In addition, it makes it very easy to miss a case when adding a new strategy. A better solution would be to define an interface that has a method for each decision or action point. Then, the Player would accept some implementation of this interfaces. This makes adding a new strategy simple and requires no changes to Player.


If you do need to make a decision based on the value of a enum, a switch statement is better than a number of if else statements. It is easier for the compiler to optimize the makes it clearer to someone readying the code that each enum value has a distinct action.

share|improve this answer

Hard coding a series of "println" statements for printing out the rules is problematical as it blurs the line between things that are code and things that are data. My preference would be to have the rules in a separate file, read it into a single string and print that string at the appropriate time. However, just moving it to a single constant string would be an improvement. Something like this:

private final String GAME_RULES = 
    "**************WELCOME TO THE GAME OF NIM******************\n" +
    "\n" +
    "\n" +
    "******************RULES OF THE GAME***********************\n" +
    "--The number of marbles in the initial pile will be random amount between 10 and 100.\n" +
    "---Each player must choose between 1 and half of the remaining pile of marbles to remove.\n" +
    "----The player stuck with the last marble loses the game.\n" +
    "---The computer will randomly be placed into simple or smart mode randomly.\n" +
    "--The player who goes first will also be chosen at random.\n" +
    "\n" +
    "Enter 1 to play against another human or 2 to play against a computer.\n" +
    "Enter 3 if you would like 2 computers to play against each other."

Then the section that is marked "Interface" becomes:

    //Interface
    System.out.println(GAME_RULES);
    selection = in.nextInt();

Which is much easier to understand because we have cleared away all the clutter.

share|improve this answer

You are using a lot of different variables to store the Player objects in and declaring them for your whole main method. If you really needed four it would be better to have an array of Players[] but in this case you are better just having 2 (maybe still in an array) but only create them when you actually need them and set them to the right type then.

By using the array that then lets you change the player 1 and player 2 while loop into one for loop to run over the array[] and you can remove all that duplicated code. It also lets you support any number of players virtually for free.

For the loop you can use a do-while loop as you know you always have to run through the loop at least once.

It would be more efficient to use one Random object created in Main and then passed into anywhere else that needs it. Other than that your Pile object looks fine.

Your Player object is a classic case where inheritance should be used.

Create an abstract base class Player and then create subclasses HumanPlayer, ComputerPlayer and SmartComputerPlayer (the computer players may or may not also have a common ancestor). Then you just have a Player object reference in your main class and it calls the relevant method in that - which gets sent to the right subclass automatically.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.