Welcome to the Java Programming Forums


The professional, friendly Java community. 21,500 members and growing!


The Java Programming Forums are a community of Java programmers from all around the World. Our members have a wide range of skills and they all have one thing in common: A passion to learn and code Java. We invite beginner Java programmers right through to Java professionals to post here and share your knowledge. Become a part of the community, help others, expand your knowledge of Java and enjoy talking with like minded people. Registration is quick and best of all free. We look forward to meeting you.


>> REGISTER NOW TO START POSTING


Members have full access to the forums. Advertisements are removed for registered users.

Results 1 to 5 of 5

Thread: Critique Java Game: Help Me Improve

  1. #1
    Junior Member
    Join Date
    Jul 2010
    Posts
    1
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Default Critique Java Game: Help Me Improve

    Hello

    I am new to Java, so I gave myself a goal of making a Console Game in Java.

    So I have finished it & I am looking for Java programmers to go over it pretty harshly & give me advice where I can improve.

    I am looking for advice on:
    - Java syntax - does my code follow correct java syntax
    - Architecture - does my code adhere to Java OOP, should some functions be removed from the player class & placed in the Game logic class etc.
    - Any "Big no no's" that I have done in my code that you can point out
    - Overall - Is this good java code



    The game I have made is a Game of Nim clone. If you dont know what Nim is, its supposably this ancient chinese strategy game, although I cant see why people have been playing this game for so long The aim is to be the person to grab the last match from a number of piles of matches.

    Download: My Java Game Source Code

    Edit: Ok I will post my code but its long...

    GameLogic.java
    /*
       ******************************************************************************************
       Nim Game
     
       Student: 
       SID:     
     
       ******************************************************************************************
       Game Logic Class:
       Controls the actual flow of game play & performs game loop.
       ******************************************************************************************
    */
     
    import java.util.*;
     
     
    public class GameLogic
    {
     
    	// Private Variables:
     
    	private enum GameStatus {ON, OFF};
    	private GameStatus gameState;
    	private boolean gameEnd;
     
    	Scanner in;
     
    	private int numOfPiles;
    	private Vector <Pile> piles;
    	private Vector <Player> players;
     
     
     
    	// Public Variables & Methods:
     
    	public GameLogic()
    	{
    		// Constructor:
     
    		gameState  = GameStatus.ON;
    		gameEnd    = false;
    		in         = new Scanner( System.in );
    		numOfPiles = 0;
    		players    = new Vector <Player>();
    		piles      = new Vector <Pile>();
     
    		registerPlayers();
    	    setPileSize();
     
    	}
     
     
    	public void initiateGameLoop()
    	{
    		// Post: Loop game until the game has finished & a winner has been announced
    		//       By using a vector of Player objects & a for loop, the game loop is 
    		//       scalable & can manage many players.
     
     
    		while ( !gameEnd )
    		{
     
    			for (int i=0; i<players.size(); i++)
    			{
     
    				int pileNum, matchNum;
    				Player selPlayer = players.elementAt(i);
     
    				// Prompt player to decide what action to take
    				if ( selPlayer instanceof Human )
    					 pileNum  = ((Human) selPlayer).selectPile( in, piles.size() - 1 );
     
    				else pileNum  = ((Computer) selPlayer).selectPile( piles.size() - 1 );
     
     
     
    				if ( pileNum < piles.size() )
    				{
    					// Remove n matches from a pile
     
    					if ( selPlayer instanceof Human )
     
    						matchNum = ((Human) selPlayer).selectMatch( in, piles.elementAt(pileNum).size() );
     
    					else matchNum = ((Computer) selPlayer).selectMatch( piles.elementAt(pileNum).size() );
     
     
    					piles.elementAt( pileNum ).remove( matchNum, selPlayer.alias );
    					selPlayer.storeLastMove( pileNum, matchNum );
     
    				}
    				else if ( pileNum == piles.size() )
    				{
    					// undo last move 
     
    					int lastMove = selPlayer.getLastMove();
    					pileNum  = (int) Math.floor(lastMove / 100);
    					matchNum = lastMove % 100;
     
    					piles.elementAt( pileNum ).replace( matchNum, selPlayer.alias );
    					selPlayer.storeLastMove( 0, 0 );
     
    				}
    				else if ( pileNum == piles.size() + 1 )
    				{
    					// redo last move
     
    					int lastMove = selPlayer.getLastMove();
    					pileNum  = (int) Math.floor(lastMove / 100);
    					matchNum = lastMove % 100;
     
    					piles.elementAt( pileNum ).remove( matchNum, selPlayer.alias );
    					selPlayer.storeLastMove( 0, 0 );
     
    				}
    				else if ( pileNum == piles.size() + 2 )
    				{
    					// clear board & restart
     
    					selPlayer.storeLastMove( 0, 0 );
    					clear();
    					i = -1;
    				}
     
     
    				printGrid();
     
     
    				// Check if the game has finished
    				if ( assessGameState() )
    				{
    					// print out that selPlayer is the winner & how many matches they have
    					printGameStats( selPlayer );
    					clear();
    					i = -1;
    				}
     
    			}
    		}
    	}
     
     
    	public void registerPlayers()
    	{
    		// Post: Prompt user to play against AI or human. (Are there 2 players
    		//       or one).
     
    		Player p1, p2;
    		int playerDecision = -1;
    		boolean validInput = false;
     
     
    		while ( !validInput )
    		{
    			try
    			{
    				System.out.printf("** No. of players ** \n1. Human VS AI \n2. Human VS Human \nEnter selection: ");
    				playerDecision = in.nextInt();
     
     
    				// Perform error checking
    				switch ( playerDecision )
    				{
    					case 1:
    						  validInput = true;
    						  p1 = new Human("White");
    						  p2 = new Computer("Black");
    						  players.add( p1 );
    						  players.add( p2 );
    					break;
    					case 2:
    						  validInput = true;
    						  p1 = new Human("White");
    						  p2 = new Human("Black");
    						  players.add( p1 );
    						  players.add( p2 );
    					break;
    					default:
    						  System.out.println("Invalid input");
    					break;
    				}
     
     
    			}
    			catch (Exception io)
    			{
    				System.out.println("Invalid input");
    				in.nextLine(); // flush input
    			}
     
    		}
     
    	}
     
     
    	public void setPileSize()
    	{
    		// Post: Prompt user to select how many piles(heaps) of matches will
    		//       be played with in the game
     
    		boolean validInput = false;
     
     
    		while ( !validInput )
    		{
    			try
    			{
    				System.out.printf("Please select the number of piles(heaps) you wish to have: ");
    				numOfPiles = in.nextInt();
     
     
    				// Perform error checking
    				if ( numOfPiles > 0 && numOfPiles <= 9 )
    				{
    					for (int i=0; i<numOfPiles; i++)
    					{
    						piles.add( new Pile() );
    					}
    					validInput = true;
    				}
    				else  System.out.println("Invalid input");
     
     
    			}
    			catch (Exception io)
    			{
    				System.out.println("Invalid input");
    				in.nextLine(); // flush input
    			}
    		}
     
    	}
     
     
    	public int printGrid()
    	{
    		// Post: Output/display every heap(pile) in the nim game
     
    		Pile selPile;
     
    		System.out.println( "\n\n*** Nim Game Board ***" );
     
    		// 10 because there are 10 positions in each pile
    		for (int i=0; i<10; i++)
    		{
    			System.out.print("  ");
    			for (int j=0; j<piles.size(); j++)
    			{
    				selPile = piles.elementAt(j);
     
    				System.out.print( selPile.at(i).value + " " );
     
    		    }
     
    			System.out.println();
    		}
     
    		System.out.println( "\n\n" );
    		return 1;
    	}
     
     
    	public void clear()
    	{
    		// Post: Clear game board & restack all piles with matches
     
    		for (int i=0; i<piles.size(); i++)
    		{
    			piles.elementAt( i ).clearHeap();
    		}
    	}
     
     
    	public boolean assessGameState()
    	{
    		// Post: Returns true if the game is finished & we have a winner
     
    		for (int i=0; i<piles.size(); i++)
    		{
    			if ( piles.elementAt(i).size() > 0 )
     
    				return false;
    		}
     
    		return true;
    	}
     
     
    	public void printGameStats( Player winner )
    	{
    		// Post: Display who is the winner & other player statistics
     
    		String winnerStats = " %s is the winner:  Total matches collected = %s \t No. of wins = %s \t No. of losses = %s \n";
    		String loserStats  = " Player Name: %s  Total matches collected = %s \t No. of wins = %s \t No. of losses = %s \n";
     
    		System.out.printf( winnerStats, winner.playerName, winner.getMatches(), 
    				           winner.incrementWin(), winner.getLosses() );
    		System.out.println(" Other Player statistics: ");
     
    		for (int i=0; i<players.size(); i++)
    		{
    			Player selPlayer = players.elementAt(i);
     
    			if ( selPlayer.playerName != winner.playerName )
    			{
    				System.out.printf( loserStats, selPlayer.playerName, selPlayer.getMatches(), 
    						           selPlayer.getWins(), selPlayer.incrementLoss() );
    			}
    		}
     
    		System.out.println("\n");
    	}
    }


    Player.java
    /*
       ******************************************************************************************
       Nim Game
     
       Student: 
       SID:     
     
       ******************************************************************************************
       Player Class:
       Uses inheritance to define the player object(both human & AI).
       ******************************************************************************************
    */
     
    import java.io.*;
    import java.util.*;
     
     
    public class Player
    {
     
    	// Private Variables:
    	protected enum PlayerType { HUMAN, AI };
    	private int matchesOwned;
    	private int lastMove;
    	private int wins;
    	private int losses;
    	public String playerName;
    	public char alias;
     
     
    	// Public Variables & Methods:
     
    	public Player( String name )
    	{
    		// Constructor:
     
    		matchesOwned = 0;
    		lastMove     = 0;
    		wins         = 0;
    		losses       = 0;
    		playerName   = name;
    		alias        = ( playerName.toUpperCase() ).charAt(0);
     
    	}
     
     
    	public String getName()
    	{
    		// Post: Return this Players name
     
    		return playerName;
     
    	}
     
     
    	public int incrementWin()
    	{
    		// Post: Increment wins & return its value
     
    		wins++;
    		return wins;
    	}
     
     
    	public int incrementLoss()
    	{
    		// Post: Increment wins & return its value
     
    		losses++;
    		return losses;
    	}
     
     
    	public void incrementMatches( int num )
    	{
    		// Post: Increment number of matches player owns
     
    		matchesOwned += num;
     
    	}
     
     
    	public int getWins()
    	{
    		// Post: Return the total no. of wins this player has
     
    		return wins;
    	}
     
     
    	public int getLosses()
    	{
            // Post: Return the total no. of times this player has lost
     
    		return losses;
    	}
     
     
    	public int getMatches()
    	{
    		// Post: Return number of matches this player owns
     
    		return matchesOwned;	
    	}
     
     
    	public void storeLastMove( int pileValue, int matchValue )
    	{
    		// Post: Store the most recent game move made by this player
     
    		lastMove = (pileValue * 100) + matchValue;
    	}
     
     
    	public int getLastMove()
    	{
    		// Post: Return the most recent game move player has made in encrypted form
     
    		return lastMove;
    	}
     
    }
     
     
    ///////////////////////////////////////////////////////////////////////////////////////////////
    //                                                                                           //
    //                            Child class of Player: Human Class                             //
    //                                                                                           //
    ///////////////////////////////////////////////////////////////////////////////////////////////
     
    class Human extends Player
    {
     
    	// Private Variables:
     
     
    	// Public Variables & Methods:
     
    	public Human(  String name )
    	{
    		// Constructor:
     
    		super( name );
    	}
     
     
    	public int selectPile( Scanner in, int numPiles )
    	{
    		// Post: Prompt user to identify which pile they want to remove matches from
     
    		char playerDecision = 'z';
    		boolean validInput = false;
     
    		while ( !validInput )
    		{
    			try
    			{
    				String pileOptions = "";
     
    				for (int i=0; i<=numPiles; i++)
    				{
    					pileOptions += i;
    					pileOptions += ',';
     
    				}
     
                    System.out.printf("Which pile does %s select (%su,r,b): ", getName(), pileOptions);
    				playerDecision = in.next().charAt(0);
     
     
    				// Perform error checking
    				if ( Character.digit(playerDecision,10) >= 0 && Character.digit(playerDecision,10) <= numPiles )
    				{
    					validInput = true;
    					return Character.digit(playerDecision,10);
    				}
    				else if ( playerDecision == 'u' || playerDecision == 'U' )
    				{
     
                        if ( getLastMove() != 0 )
     
    						return numPiles + 1;
     
    				}
    				else if ( playerDecision == 'r' || playerDecision == 'R' )
    				{
     
    					if ( getLastMove() != 0 )
     
    						return numPiles + 2;
     
    				}
    				else if ( playerDecision == 'b' || playerDecision == 'B' )
    				{
    					return numPiles + 3;
    				}
     
                    System.out.println("Invalid input");
     
    			}
    			catch (Exception io)
    			{
    				System.out.println("Invalid input");
    				in.nextLine(); // flush input
    			}
    		}
     
    		return playerDecision;
     
    	}
     
     
    	public int selectMatch( Scanner in, int maxRemoval )
    	{
    		// Post: Prompt user to select x many matches to remove from a pile
     
    		int matchRemoveNum = 0;
            boolean validInput = false;
     
     
    		while ( !validInput )
    		{
    			try
    			{
    				System.out.printf("How many matches do you wish to remove: ");
    				matchRemoveNum = in.nextInt();
     
    				if ( matchRemoveNum <= maxRemoval )
     
    					validInput = true;
     
    				else System.out.printf("You cant remove that many. Maximum is %s. \n", maxRemoval);
     
    			}
    			catch (Exception io)
    			{
    				System.out.println("Invalid input");
    				in.nextLine(); // flush input
    			}
    		}
     
    		incrementMatches( matchRemoveNum );
     
    		return matchRemoveNum;
    	}
     
    }
     
     
    ///////////////////////////////////////////////////////////////////////////////////////////////
    //                                                                                           //
    //                         Child class of Player: Computer Class                             //
    //                                                                                           //
    ///////////////////////////////////////////////////////////////////////////////////////////////
     
    class Computer extends Player
    {
     
    	// Private Variables:
     
     
     
    	// Public Variables & Methods:
     
    	public Computer( String name )
    	{
    		// Constructor:
     
    		super( name );
    	}
     
     
    	public int selectPile( int numPiles )
    	{
    		// Post: Get computer to randomly select a pile to remove matches from
     
            return (int) (Math.random() * numPiles);
    	}
     
     
    	public int selectMatch( int maxRemoval )
    	{
    		// Post: Get computer to randomly select how many matches to remove
     
    		int matchRemoveNum = (int) (Math.random() * maxRemoval);
     
    		incrementMatches( matchRemoveNum );
     
    		return matchRemoveNum;
    	}
     
    }


    Pile.java
    /*
       ******************************************************************************************
       Nim Game
     
       Student: 
       SID:     
     
       ******************************************************************************************
       Pile & Match Class:
       ...
       ******************************************************************************************
    */
     
    import java.util.*;
     
     
    public class Pile
    {
     
    	// Private Variables:
     
    	private Vector <Match> myPile;
    	private int freeMatches;
     
    	public class Match
    	{
    		public char value;
     
    		public Match()
    		{
    			value = '*';
    		}
     
    	}
     
     
     
    	// Public Variables & Methods:
     
    	public Pile()
    	{
    		// Constructor:
     
    		myPile      = new Vector <Match>();
    		freeMatches = 10;
     
    		for (int i=0; i<10; i++)
    		{
    			myPile.add( new Match() );
    		}
    	}
     
     
    	public int capacity()
    	{
    		// Post: Return the capacity of this pile
     
    		return myPile.size();
    	}
     
    	public int size()
    	{
            // Post: Return the remaining matches in this pile
     
    		return freeMatches;
    	}
     
     
    	public Match at( int index )
    	{
    		// Post: Return the Match object at the specified index
     
    		if ( index >= 0 && index < myPile.size() )
    		{
    			return myPile.elementAt( index );
    		}
    		else return null;
     
    	}
     
     
    	public int remove( int nMatches, char alias )
    	{
    		// Post: Remove n Matches from the pile
     
    		for (int i = 0, index = freeMatches-1; i<nMatches; i++, index--)
    		{
    			myPile.elementAt( index ).value = alias;
    			freeMatches--;
    		}
     
    		return freeMatches;
     
    	}
     
     
    	public int replace(  int nMatches, char alias )
    	{
    		// Post: Replace n Matches back into the pile
     
    		for (int i = 0, index = freeMatches; i<nMatches; i++, index++)
    		{
    			myPile.elementAt( index ).value = '*';
    			freeMatches++;
    		}
     
    		return freeMatches;
     
    	}
     
     
    	public void clearHeap()
    	{
    		// Post: Replace all matches in pile(heap)
     
    		while ( freeMatches != this.capacity() )
    		{
    			myPile.elementAt( freeMatches ).value = '*';
    			freeMatches++;
    		}
    	}
     
    }
    Last edited by gretty; July 14th, 2010 at 04:40 AM.


  2. #2
    Super Moderator helloworld922's Avatar
    Join Date
    Jun 2009
    Posts
    2,896
    Thanks
    23
    Thanked 619 Times in 561 Posts
    Blog Entries
    18

    Default Re: Critique Java Game: Help Me Improve

    hmm... from a first look-over, you may want to consider using Javadoc comments. This way you can automatically generate an API doc for someone to look at. It also allows certain IDE's (such as Eclipse or NetBeans) to provide documentation tooltips when you are coding.

    Java doc looks like this:

    /**
     * Notice how the javadoc starts with two asterisks instead of 1 for normal block comments
     */

    You can also add various annotations to make the javadoc more helpful for users.

    /**
     * This method adds two numbers
     *
     * @param num1
     *                        The first number
     * @param num2
     *                        The second number
     * @return the sum of num1 and num2
     */
    public static int addem(int num1, num2)
    {
         return num1 + num1;
    }

    Javadoc also give you a place to layout a method/class contract, i.e. things that can be guaranteed when using that object (or, at least things that are suppose to be guaranteed), as well as any special formatting of results/input, or the behavior of the method/class in "strange" situations.

    /**
     * Adds two numbers. If either number is less than 0, returns -1.
     */
    public static int addem(int num1, int num2)
    {
        if(num1 < 0 || num2 < 0)
        {
            return -1;
        }
        return num1 + num2;
    }

    That's the only major thing I can see right off the bat without any thorough analysis, I'm assuming the game runs and works correctly (if it doesn't, this would be a problem. I didn't actually try to compile or run it).

  3. #3
    Administrator copeg's Avatar
    Join Date
    Oct 2009
    Location
    US
    Posts
    5,320
    Thanks
    181
    Thanked 833 Times in 772 Posts
    Blog Entries
    5

    Default Re: Critique Java Game: Help Me Improve

    Ditto on the subject of javadoc comments. They can be quite useful. Other more minor things (and I'll mention that I didn't try and compile the code either):

    - You may wish to declare Person abstract, defining the methods used in both the child classes selectPile and selectMatch as abstract. This would allow you to remove the instanceof's and just call the method on Player (and is better design imo because any addition classes written that extend Player wouldn't necessitate changing the code of all the instanceof's)

    - I saw a few public variables which you may wish to encapsulate and make private.

    - Minor point, but you may wish to use an ArrayList rather than a Vector (Vector is synchronized and ArrayList not)
    Last edited by copeg; July 14th, 2010 at 07:25 PM.

  4. #4
    Banned
    Join Date
    May 2010
    Location
    North Central Illinois
    Posts
    1,631
    My Mood
    Sleepy
    Thanks
    390
    Thanked 112 Times in 110 Posts

    Default Re: Critique Java Game: Help Me Improve

    You need a main method in class GameLogic for the program to work.

  5. #5
    Super Moderator Norm's Avatar
    Join Date
    May 2010
    Location
    Eastern Florida
    Posts
    25,042
    Thanks
    63
    Thanked 2,708 Times in 2,658 Posts

    Default Re: Critique Java Game: Help Me Improve

    The use of the hardcoded literals: 100 about line 97 aren't explained.
    Would a final int be better then the name would self document

    What is the logic about comparing: pileNum vs piles.size()
    There are several comparisons made but not documented.

    As mentioned above, the method selectPile() needs to be redesigned. The following looks awkard:
    				if ( selPlayer instanceof Human )
    					 pileNum  = ((Human) selPlayer).selectPile( in, piles.size() - 1 );
     
    				else pileNum  = ((Computer) selPlayer).selectPile( piles.size() - 1 );
    Last edited by Norm; July 21st, 2010 at 08:12 PM.

Similar Threads

  1. Help With Java Game Code
    By CheekySpoon in forum What's Wrong With My Code?
    Replies: 0
    Last Post: April 26th, 2010, 04:07 PM
  2. Snake game in java
    By freaky in forum Object Oriented Programming
    Replies: 0
    Last Post: April 19th, 2010, 11:04 AM
  3. Need help for my java game
    By blunderblitz in forum What's Wrong With My Code?
    Replies: 1
    Last Post: March 27th, 2010, 05:32 AM
  4. Simple game in Java
    By velop in forum What's Wrong With My Code?
    Replies: 1
    Last Post: March 27th, 2010, 05:04 AM
  5. Help with 1st Java applet game!
    By Sneak in forum Java Applets
    Replies: 0
    Last Post: November 28th, 2009, 11:20 AM