Critique Java Game: Help Me Improve

  • (2 Pages)
  • +
  • 1
  • 2

22 Replies - 2964 Views - Last Post: 16 July 2010 - 11:03 PM Rate Topic: -----

#1 gretty  Icon User is offline

  • D.I.C Head

Reputation: 3
  • View blog
  • Posts: 123
  • Joined: 25-May 09

Critique Java Game: Help Me Improve

Posted 13 July 2010 - 11:45 PM

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
/*
   ******************************************************************************************
   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++;
		}
	}
	
}



This post has been edited by gretty: 14 July 2010 - 02:32 AM


Is This A Good Question/Topic? 0
  • +

Replies To: Critique Java Game: Help Me Improve

#2 bcranger  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 252
  • View blog
  • Posts: 1,199
  • Joined: 01-February 10

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 12:23 AM

Please post your code as we have been attacked by malicious downloads in the past :)
Was This Post Helpful? 0
  • +
  • -

#3 gretty  Icon User is offline

  • D.I.C Head

Reputation: 3
  • View blog
  • Posts: 123
  • Joined: 25-May 09

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 04:41 AM

what no replies :(

Please give me some advice
Was This Post Helpful? 0
  • +
  • -

#4 sh1n3  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 24
  • View blog
  • Posts: 164
  • Joined: 22-April 10

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 05:45 AM

- Java syntax - 9/10, for nice use of Enumerations and Generics
- Architecture - 9/10
- Any "Big no no's" - NA
- Overall - A nice game, a 18/20

**Try adding a save/load method or a timer to make it more interesting
Was This Post Helpful? 0
  • +
  • -

#5 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 2388
  • View blog
  • Posts: 5,012
  • Joined: 11-December 07

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 05:55 AM

A lot of your comments are uneccesary.

///////////////////////////////////////////////////////////////////////////////////////////////
//                                                                                           //
//                            Child class of Player: Human Class                             //
//                                                                                           //
///////////////////////////////////////////////////////////////////////////////////////////////

class Human extends Player



The line of code that follows has exactly the same info as your comments.

	public Human(  String name )
	{
		// Constructor:
		
		super( name );
	}



Yes, that's what a constructor looks like. If I were writing that code, it would look like this:
	public Human(String name) {super(name);}


	// Private Variables:
	
	
	// Public Variables & Methods:



You have no private members. Why the comment? I know the public ones are public because their declarations tell me.

	public int incrementWin()
	{
		// Post: Increment wins & return its value
		
		wins++;
		return wins;
	}



Here the comment IS necessary because it's not obvious from the method name what is being returned. You can easily fix that by renaming the method:

public int incrementAndGetWin() {
    wins++;
    return wins;
}

// Or if it suits your tastes:

public int incrementAndGetWin() {return ++wins;}


On a similar note, don't double space your code. It makes it harder to read. Use line breaks to break chunks of code to make them easier to read.

Here is how I would have laid out your Player class. Hopefully this will show you how more straightforward your code could be. I don't think I've crammed it up too small and you can see everything at a glance:

public class Player {

	protected enum PlayerType { HUMAN, AI }

	private int matchesOwned;
	private int lastMove;
	private int wins;
	private int losses;
	
	public String playerName;
	public char alias;



	public Player( String name ){
		matchesOwned = 0;
		lastMove     = 0;
		wins         = 0;
		losses       = 0;
		playerName   = name;
		alias        = ( playerName.toUpperCase() ).charAt(0);
	}

	public String getName()     {return playerName;}
	public int    getWins()     {return wins;}
	public int    getLosses()   {return losses;}
	public int    getMatches()  {return matchesOwned;}
	public int    getLastMove() {return lastMove;}

	public int incrementAndGetWin()  {return ++wins;}
	public int incrementAndGetLoss() {return ++losses;}

	public void incrementMatches( int num ) {matchesOwned += num;}

	public void storeLastMove( int pileValue, int matchValue ) {
		lastMove = (pileValue * 100) + matchValue;
	}

}


As for the design and the code itself, nothing really jumps out. But at the moment, it's kinda hard to trawl through.

This post has been edited by cfoley: 14 July 2010 - 05:57 AM

Was This Post Helpful? 0
  • +
  • -

#6 Dogstopper  Icon User is offline

  • The Ninjaducky
  • member icon

Reputation: 2965
  • View blog
  • Posts: 11,222
  • Joined: 15-July 08

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 09:23 AM

cfoley makes a controversial comment there about comments. I agree with him that comments are quite often unnecessary if you have good variable and method names. Often, exorbitant comments actually get in the way of good production-level code. When one tries to find something,big frilly comments just get in the way.

However, he also suggested one-line methods, which are fine, but I personally don't like them.
Was This Post Helpful? 0
  • +
  • -

#7 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 2388
  • View blog
  • Posts: 5,012
  • Joined: 11-December 07

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 10:30 AM

There are lots of things in the class I posted above that are personal preference. I wouldn't always lay out a class like that, but sometimes it does make things clearer. Certainly, I feel that getters and setters are best presented as above. (though javadoc does screw that up!)

Comments certainly have their uses, but they are best reserved for when you do something out of the ordinary or that needs explaining. For example, I often comment bug fixes. Sometimes the fix can look messy and, from experience, I have been known to inadvertantly remove the fix if I'm cleaning a section of code.
Was This Post Helpful? 0
  • +
  • -

#8 pbl  Icon User is offline

  • There is nothing you can't do with a JTable
  • member icon

Reputation: 8378
  • View blog
  • Posts: 31,956
  • Joined: 06-March 08

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 04:36 PM

A "Big No No" for the lack of comments and the absence of the most basic JavaDoc
Was This Post Helpful? 0
  • +
  • -

#9 gretty  Icon User is offline

  • D.I.C Head

Reputation: 3
  • View blog
  • Posts: 123
  • Joined: 25-May 09

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 04:57 PM

Thanks for all the advice :)


View Postpbl, on 14 July 2010 - 03:36 PM, said:

A "Big No No" for the lack of comments and the absence of the most basic JavaDoc


I have never heard of a JavaDoc, what is it? I want to learn more about java - should a JavaDoc be included in every class/project?
Was This Post Helpful? 0
  • +
  • -

#10 pbl  Icon User is offline

  • There is nothing you can't do with a JTable
  • member icon

Reputation: 8378
  • View blog
  • Posts: 31,956
  • Joined: 06-March 08

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 05:10 PM

View Postgretty, on 14 July 2010 - 05:57 PM, said:

Thanks for all the advice :)


View Postpbl, on 14 July 2010 - 03:36 PM, said:

A "Big No No" for the lack of comments and the absence of the most basic JavaDoc


I have never heard of a JavaDoc, what is it? I want to learn more about java - should a JavaDoc be included in every class/project?

You have consulted the API some times ?
Do you think somebody wrote all that ? All the name of the methods, all the name of the parameters, a small resume of what it does ? The name of the methods accessible in the father class ? The HTML link on the left side to go to the class you want ?

Nobody typed it it is actually in the code. If you comment your code, on a certain matter, you run javadoc and all these .html files are made for you.

Google javadoc
Was This Post Helpful? 0
  • +
  • -

#11 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 2388
  • View blog
  • Posts: 5,012
  • Joined: 11-December 07

Re: Critique Java Game: Help Me Improve

Posted 14 July 2010 - 05:32 PM

Javadoc is awesome if you're writing an API. :)
Was This Post Helpful? 0
  • +
  • -

#12 YasuoDancez  Icon User is offline

  • D.I.C Head

Reputation: 20
  • View blog
  • Posts: 135
  • Joined: 30-September 09

Re: Critique Java Game: Help Me Improve

Posted 15 July 2010 - 11:32 PM

^ I highly doubt JavaDoc is only good for writing an API if that is what you are saying.

The way I have been taught is that you should comment everything.
Since my year of deep in depth day to day programming and learning, I haven't heard someone say that you can overuse comments.
I believe comments do not hurt and I would rather have enough or more than enough than little or none.

Back on topic, this game project looks like a nice one. I think I shall give it a try and maybe I can post my code when done to compare. Don't hold your breath it may take a week or more.

Gretty, how long did it take for you to finish this project and where did you get the project from?
A book or class? I would like to have the case study/ project info for this.

This post has been edited by YasuoDancez: 15 July 2010 - 11:38 PM

Was This Post Helpful? 0
  • +
  • -

#13 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 2388
  • View blog
  • Posts: 5,012
  • Joined: 11-December 07

Re: Critique Java Game: Help Me Improve

Posted 16 July 2010 - 12:30 AM

It's not that I believe comments are bad, it's just that it's best to make them without using comment lines. Here is an article which echos my point of view far better than I ever could:
http://www.codinghor...t-comments.html

And another excellent article from the same site:
http://www.codinghor...nts-go-bad.html

Part of the problem with over use of comments is that programmers are less likely to maintain the comments than the code. There is an example in this game:

	// 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:



Here, some of the private variables have been changed to public (which is fine) but not moved to the right section (which makes the comment incorrect). Since very little is gained by this particular comment in the first place, it's better to leave it out than to have the maintenence overhead.

Sorry for labouring my point gretty. I'll put your code into my IDE later and have a good look for you.
Was This Post Helpful? 0
  • +
  • -

#14 YasuoDancez  Icon User is offline

  • D.I.C Head

Reputation: 20
  • View blog
  • Posts: 135
  • Joined: 30-September 09

Re: Critique Java Game: Help Me Improve

Posted 16 July 2010 - 03:16 AM

Yeah it seems you have a good point there.
Just saying
//Instance variables 


would have been good enough and saying private is not really needed.

Quote

Part of the problem with over use of comments is that programmers are less likely to maintain the comments than the code.

Comment anonymous for the comment junkies :D
Was This Post Helpful? 0
  • +
  • -

#15 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 2388
  • View blog
  • Posts: 5,012
  • Joined: 11-December 07

Re: Critique Java Game: Help Me Improve

Posted 16 July 2010 - 03:32 AM

Haha! I used to comment every method and every instance variable. I used to javadoc even the private methods in classes which were only visible to the package. Then I realised I was spending more time writing comments than coding, that the comments were unhelpful at best, misleading at worse.

private int notchesInBedpost;


The syntax of this line tells me that it is an instance variable and that it is private. Its name tells me it is how often I have got lucky. As a bonus, its value will be higher without the comment. :P
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2