Results 1 to 2 of 2
  1. #1
    gretty is offline Member
    Join Date
    Jul 2010
    Posts
    2
    Rep Power
    0

    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 :P 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
    Java Code:
    /*
       ******************************************************************************************
       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
    Java Code:
    /*
       ******************************************************************************************
       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
    Java Code:
    /*
       ******************************************************************************************
       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; 07-14-2010 at 11:59 AM. Reason: Added source code

  2. #2
    al_Marshy_1981 is offline Senior Member
    Join Date
    Feb 2010
    Location
    Waterford, Ireland
    Posts
    748
    Rep Power
    5

    Default

    You are on the right track, I would do away with the big star ceremony comments however, over doing it. Comments that state the obvious are not really needed like //private variables, unless you are doing some college course, and have to hand feed a lecturer.
    ~One thing I always do with booleans is add the 'is' before its variable name so like instead of say:

    boolean lightOn;

    I would do:

    boolean isLightOn;

    pedantic but adds alot because like boolean must be one or the other.

Similar Threads

  1. Improve my GUI!
    By AJArmstron@aol.com in forum New To Java
    Replies: 8
    Last Post: 04-27-2010, 10:17 PM
  2. how to improve my security?
    By anthrax in forum JavaServer Pages (JSP) and JSTL
    Replies: 1
    Last Post: 03-13-2009, 10:08 AM
  3. Replies: 16
    Last Post: 08-05-2008, 12:34 PM
  4. Replies: 25
    Last Post: 06-28-2008, 06:08 PM
  5. how to improve java performance
    By sunjavaboy in forum Advanced Java
    Replies: 6
    Last Post: 06-15-2008, 06:58 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •