Results 1 to 10 of 10
  1. #1
    Zeramat is offline Member
    Join Date
    Jun 2011
    Posts
    5
    Rep Power
    0

    Default Requesting Code Review

    I am in the process of trying to learn Java by reading a book. In general I learn better by doing than by reading, and the book has had several chapters without solid examples, so I decided to create my own problem. To that end I have written an implementation of the “Hunt the Wumpus” game originally published by Gregory Yob in 1973.

    What I am looking for is comments on how to improve the program, both in terms of Object Orientation, and usage of the Java language. In particular I find the inputAction method of the Interface class to be inelegant, mostly due to handling various error condtions. Any and all comments are welcome.

    Thank you for your time.

    wumpus.java
    Java Code:
    // This program is free software: you can redistribute it and/or modify it under the terms 
    // of the GNU General Public License as published by the Free Software Foundation; either 
    // version 3 of the License, or (at your option) any later version.
    // 
    // This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
    // without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
    // See the GNU General Public License for more details.
    // 
    // A copy of the GNU General Public License can be found at: http://www.gnu.org/licenses/.
    
    package JavaAIO;
    
    import java.io.IOException;
    
    enum Actions {MOVE, SHOOT, QUIT};  // possible actions of a player.
    
    public class Wumpus // Hunt the Wumpus game
    {
    	public static void main(String[] args)throws IOException // Main
    	{
    		Interface.displayIntroduction();
    		do
    		{
    			playGame();
    		}while (Interface.playAgain());
    		Interface.displayGoodbye();
    	}
    	
    	public static void playGame() throws IOException // Main game loop.
    	{
    		Cave cave = new Cave(); // The collection of caves. 
    		Move move = new Move(); // The action (input) from the player.
    		Player player = new Player(cave); // The player.
    		boolean fGameOver = false; // Flag for end of game.
    		
    		do
    		{
    			cave.displayRoom(player.getRoom());
    			move = Interface.inputAction();
    			switch (move.getAction())
    			{
    			case MOVE:
    				player.moveToRoom(move.getRoom(), cave);
    				break;
    			case SHOOT:
    				player.shootIntoRoom(move.getRoom(), cave);
    				if (cave.isWumpus(-1)) // Is the wumpus dead?
    					fGameOver = true;
    				break;
    			case QUIT:
    				fGameOver = true;
    				break;
    			}
    			if (cave.isWumpus(player.getRoom())) // Are the wumpus and player in the same room?
    			{
    				Interface.displayLoseWumpus();
    				fGameOver = true;
    			}
    			if (!player.isAlive()) // Was the player killed?
    				fGameOver = true;
    		} while(!fGameOver);
    	}
    }
    interface.java
    Java Code:
    package JavaAIO;
    
    import java.io.*;
    import java.util.*;
    
    public class Interface // Collection of input and output methods
    {
    	static Scanner sc = new Scanner(System.in);
    	static boolean fDebug = true; // Flag for displaying debug messages
    
    	public static Move inputAction() throws IOException // get a move from the user.
    	{
    		boolean fValid = false; // Flag to indicate we have valid input.
    		Move move = new Move(); // The users action.
    		
    		BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
    		
    		do
    		{
    			String line = "";  // The input line.
    			int tokens = 0; // Number of "tokens" on the input line.
    			int index = 0; // The token we are currently on.
    			
    			System.out.print("Move or Shoot?: ");
    			line = in.readLine();
    			String[] asTokens = line.split("\\s");
    			tokens = asTokens.length;
    			if (line.length() != 0) // if they just hit return don't do anything.
    			{
    				try // see if they just entered a room number (if so move there).
    				{
    					move.setRoom(Integer.parseInt(asTokens[index]));
    					move.setAction(Actions.MOVE);
    					fValid = true;
    				}
    				catch (NumberFormatException e)
    				{
    					char cFirst = Character.toUpperCase(asTokens[index].charAt(0));
    					switch(cFirst)
    					{
    					case 'M' : // Move
    						if (tokens > 1) // Did they already supply a room number?
    						{
    							try // Make sure it was a number.
    							{
    								move.setRoom(Integer.parseInt(asTokens[++index]));
    								move.setAction(Actions.MOVE);
    								fValid = true;
    							}
    							catch (NumberFormatException e2)
    							{
    							}
    						}
    						else
    						{
    							System.out.print("To which room?: ");
    							try // Make sure it is a number.
    							{
    								move.setRoom(sc.nextInt());
    								move.setAction(Actions.MOVE);
    								fValid = true;
    								sc.nextLine(); // Needed to eat the <return> left in the input stream after nextInt.
    							}
    							catch (InputMismatchException e2)
    							{
    								sc.nextLine(); // Needed to eat the input if it was not valid.
    							}
    						}
    						break;
    					case 'S' : // Shoot
    						if (tokens > 1) // Did they already supply a room number?
    						{
    							try // Make sure it was a number.
    							{
    								move.setRoom(Integer.parseInt(asTokens[++index]));
    								move.setAction(Actions.SHOOT);
    								fValid = true;
    							}
    							catch (NumberFormatException e2)
    							{
    							}
    						}
    						else
    						{
    							System.out.print("Into which room?: ");
    							try // make sure it is a number.
    							{
    								move.setRoom(sc.nextInt());
    								move.setAction(Actions.SHOOT);
    								fValid = true;
    								sc.nextLine(); // needed to eat the <return> left in the input stream after nextInt.
    							}
    							catch (InputMismatchException e2)
    							{
    								sc.nextLine(); // needed to eat the input if it was not valid.
    							}
    						}
    						break;
    					case 'Q' : // Quit
    						move.setAction(Actions.QUIT);
    						fValid = true;
    						break;
    					default :	
    					}
    				}
    			}
    			if (!fValid)
    				System.out.println("Try agian");
    		}while (!fValid);
    		
    		return move;
    	}
    	
    	public static boolean playAgain() // Ask if the player want to play another game.
    	{
    		String line; // The input line.
    		boolean fAgain = false; // Flag for if the player wants to play again.
    		boolean fValid = false; // Flag for valid input
    
    		do
    		{
    		System.out.print("Would you like to play another game? (Y/N) :");
    		line = sc.nextLine();
    		if (line.length() != 0) // Make sure they just did not hit return.
    		{
    			char cFirst = Character.toUpperCase(line.charAt(0));
    			if (cFirst == 'Y')
    			{
    				fAgain = true;
    				fValid = true;
    			}
    			else if (cFirst == 'N')
    			{
    				fAgain = false;
    				fValid = true;
    			}
    			else
    				fValid = false;
    		}
    		else
    			fValid = false;
    		} while (!fValid);
    				
    		return fAgain;
    	}
    
    	public static void displayRoom(int room, int[] neighbors) // display room and neighbors.
    	{
    		System.out.println();
    		System.out.print("You are in room " + room + " there are tunnels to ");
    		for (int index = 0; index < 3; index++)
    			System.out.print(neighbors[index] + " ");
    		System.out.println();
    	}
    	
    	public static void displayPit() // Pit warning.
    	{
    		System.out.println("You feel a draft.");
    	}
    	
    	public static void displayBats() // Bats warning.
    	{
    		System.out.println("You hear squeaking.");
    	}
    	
    	public static void displayWumpus() // Wumpus warning.
    	{
    		System.out.println("You smell a wumpus.");
    	}
    	
    	public static void displayBatsMove() // Player got snatched by bats!
    	{
    		System.out.println("Zap – Super bat snatch!  Elsewhereville for you!");
    	}
    
    	public static void displayBadMove() // Player tried to move into a invalid room.
    	{
    		System.out.println("You bang your head on the wall");
    	}
    	
    	public static void displayBadShot() // Player shot into a invalid room.
    	{
    		System.out.println("Your arrow bounces off the wall and almost hits you!");
    	}
    	
    	public static void displayWin() // Player killed the wumpus.
    	{
    		System.out.println("You killed the Wumpus!  You win!");
    	}
    	
    	public static void displayLosePit() // Player fell into a pit.
    	{
    		System.out.println("YYYIIIIEEEE. . . You walked into a pit!  You lose.");
    	}
    
    	public static void displayMoveWumpus() // Let the player know they walked in on the wumpus.
    	{
    		System.out.println("You stumbled into the Wumpus’s lair, and woke him!");
    	}
    	
    	public static void displayLoseWumpus() // Wumpus had a lite snack!
    	{
    		System.out.println("The Wumpus ate you!  You lose.");
    	}
    	
    	public static void displayMiss(int arrows) // Missed the wumpus.
    	{
    		System.out.println("Your shot missed!  The noise from your arrow woke the Wumpus, and he may have moved.");
    		System.out.println("You have " + arrows + " arrow(s) left.");
    	}
    	
    	public static void displayOutOfArrows() // Out of arrows.
    	{
    		System.out.println("You used your last arrow!  You lose.");
    	}
    	
    	public static void displayGoodbye() // Say Goodbye.
    	{
    		System.out.println("Thank you for playing.");
    	}
    	
    	public static void displayIntroduction() // Say Hello.
    	{
    		System.out.println("Hunt the Wumpus");
    		System.out.println();
    	}
    	
    	public static void displayDebugSetup(int wumpus, int pitOne,  // Debug info
    										 int pitTwo, int batsOne, 
    										 int batsTwo)
    	{
    		if (fDebug)
    		{
    			System.out.print("(debug) The Wumpus is in room " + wumpus);
    			System.out.print(" Pits are in rooms " + pitOne + " and " + pitTwo);
    			System.out.println(" Bats are in rooms " + batsOne + " and " + batsTwo);
    		}
    	}
    	
    	public static void displayDebugWumpus(int wumpus) // Debug for if the wumpus moves.
    	{
    		if(fDebug)
    			System.out.println("(debug) The Wumpus is now in room " + wumpus);
    	}
    }
    cave.java
    Java Code:
    package JavaAIO;
    
    public class Cave  // Implementation of the cave complex.
    {
    // -1 value means uninitialized or dead (in the case of the wumpus).
    	int wumpus = -1; // Location of the wumpus.
    	int pitOne = -1; // Location of the first pit.
    	int pitTwo = -1; // Location of the second pit.
    	int batsOne = -1; // Location of the first set of bats.
    	int batsTwo = -1; // Location of the second set of bats.
    	
    	static int[][] topology = { // array of rooms, and what rooms they connect to.
    		{3,13,17},{6,14,16},{3, 7,18},{0, 2,16},{6,18,19},
    		{8, 9,11},{1, 4,15},{2,12,19},{5,10,13},{5,11,17},
    		{8,14,16},{5, 9,18},{7,14,15},{0, 8,15},{1,10,12},
    		{6,12,13},{1, 3,10},{0, 9,19},{2, 4,11},{4, 7,17}};
    	
    	public Cave()  // Initialize the cave complex.
    	{
    		wumpus = getEmptyRoom();
    		pitOne = getEmptyRoom();
    		pitTwo = getEmptyRoom();	
    		batsOne = getEmptyRoom();
    		batsTwo = getEmptyRoom();
    
    		Interface.displayDebugSetup(wumpus, pitOne, pitTwo, batsOne, batsTwo);
    	}
    	
    	public int getEmptyRoom() // find a room that has no other occupant.
    	{
    		int room; // The room.
    		
    		do
    		{
    			room = (int) (Math.random() * 20);
    		} while ((room == wumpus) || (room == pitOne) ||
    			     (room == pitOne) || (room == batsOne) ||
    			     (room == batsTwo));
    
    		return room;
    	}
    	
    	public int getRandomRoom() // generate a random room.
    	{
    		return((int)(Math.random() * 20));
    	}
    	
    	public boolean validateRoom(int first, int second) // test to see if there is a connection between two rooms.
    	{
    		boolean fValid = false; // flag for if a valid room was found.
    		
    		for (int index = 0; index < 3; index++)
    			if (topology[first][index] == second)
    				fValid = true;
    		
    		return fValid;
    	}
    	
    	public void moveWumpus() // if the wumpus wakes up, 75% of the time move him to a new room.
    	{
    		double r = Math.random();
    		
    		if (r < .75)
    		{
    			wumpus = topology[wumpus][(int) (Math.random() * 3)];
    		}
    		Interface.displayDebugWumpus(wumpus);
    	}
    
    	public void killWumpus() // Set the wumpus to be dead.
    	{
    		wumpus = -1;
    	}
    	
    	public boolean isWumpus(int room) // Test to see room contains the wumpus.
    	{
    		return(room == wumpus);
    	}
    	
    	public boolean isPit(int room) // Test to see if the room contains a pit.
    	{
    		return ((room == pitOne) || (room == pitTwo));
    	}
    
    	public boolean isBats(int room) // Test to see if the rooms contains bats.
    	{
    		return ((room == batsOne) || (room == batsTwo));
    	}
    	
    	public boolean isAdjacentPit(int room) // Test to see if the room is next to a pit.
    	{
    		boolean fPit = false; // Flag for if a pit was found.
    		
    		for (int index = 0; index < 3; index++)
    			if (this.isPit(topology[room][index]))
    				fPit = true;
    		return fPit;
    	}
    	
    	public boolean isAdjacentBats(int room) // Test to see of the room is next to bats.
    	{
    		boolean fBats = false; // Flag for if bats were found.
    		
    		for (int index = 0; index < 3; index++)
    			if (this.isBats(topology[room][index]))
    				fBats = true;
    		return fBats;
    	}
    	
    	public boolean isNearWumpus(int room) // Test to see if the room is within two rooms of a wumpus.
    	{
    		boolean fWumpus = false; // Flag for if a wumpus was found.
    
    		for (int index = 0; index < 3; index++)
    		{
    			if (this.isWumpus(topology[room][index]))
    				fWumpus = true;
    			if (this.isAdjacentWumpus(topology[room][index]))
    				fWumpus = true;
    		}
    		return fWumpus;
    	}
    	
    	public boolean isAdjacentWumpus(int room) // Test to see if the room in next to a wumpus.
    	{
    		boolean fWumpus = false; // Flag for if a wumpus was found.
    		
    		for (int index = 0; index < 3; index++)
    			if (this.isWumpus(topology[room][index]))
    				fWumpus = true;
    		return fWumpus;
    	}
    	
    	public void displayRoom(int room) // Display information about the selected room.
    	{
    		Interface.displayRoom(room, topology[room]);
    		if (isAdjacentPit(room))
    			Interface.displayPit();
    		if (isAdjacentBats(room))
    			Interface.displayBats();
    		if (isNearWumpus(room))
    			Interface.displayWumpus();		
    	}
    }
    move.java
    Java Code:
    package JavaAIO;
    
    public class Move // Class to collect player action and room into one entity.
    {
    	private Actions action; // What action the player is taking.
    	private int room; // What room that action is on.
    	
    	public Move() // Initialize the move.
    	{
    		action = Actions.QUIT;
    		room = -1;
    	}
    	
    	public void setAction(Actions a) // Set the action.
    	{
    		action = a;
    	}
    	
    	public void setRoom(int r) // Set the room.
    	{
    		room = r;
    	}
    
    	public Actions getAction() // Retrieve the action.
    	{
    		return action;
    	}
    	
    	public int getRoom() // Retrieve the room.
    	{
    		return room;
    	}
    }
    player.java
    Java Code:
    package JavaAIO;
    
    public class Player // Implementation of the Wumpus Hunter.
    {
    	private int room;  // Room the player is currently in.
    	private int arrows; // Number of arrows the player has left.
    	private boolean fIsAlive; // Flag for if the player is alive or not.
    	
    	public Player(Cave cave) // Initialize the player.
    	{
    		room = cave.getEmptyRoom();
    		arrows = 5;
    		fIsAlive = true;
    	}
    	
    	public int getRoom()  // Return the room the player is in.
    	{
    		return room;
    	}
    	
    	public boolean isAlive() // Test to see if the player is alive.
    	{
    		return fIsAlive;
    	}
    	
    	public void moveToRoom(int room, Cave cave) // Move the player to a new room.
    	{
    		if (cave.validateRoom(this.room, room)) // Is there a connection between rooms?
    		{
    			this.room = room;
    			if (cave.isBats(room)) // Does this room have bats?
    			{
    				this.room = cave.getRandomRoom();
    				Interface.displayBatsMove();
    			}
    			if (cave.isWumpus(room)) // Does this room have the wumpus?
    			{
    				Interface.displayMoveWumpus();
    			}
    			if (cave.isPit(room)) // Does this room have a pit?
    			{
    				Interface.displayLosePit();
    				fIsAlive = false;
    			}
    		}
    		else
    			Interface.displayBadMove();
    	}
    	
    	public void shootIntoRoom(int room, Cave cave) // Shoot an arrow into a room.
    	{
    		arrows--;
    		if (cave.validateRoom(this.room, room)) // Is there a connection between rooms?
    			if (cave.isWumpus(room)) // Did the arrow hit the wumpus?
    			{
    				Interface.displayWin();
    				cave.killWumpus();
    			}
    			else
    			{
    				Interface.displayMiss(arrows);
    				cave.moveWumpus();
    			}
    		else
    			Interface.displayBadShot();
    		if ((arrows == 0)  && (!cave.isWumpus(-1))) // Is the player out of arrows? (and wumpus still alive)
    		{
    			Interface.displayOutOfArrows();
    			fIsAlive = false;
    		}
    	}
    }

  2. #2
    Junky's Avatar
    Junky is offline Grand Poobah
    Join Date
    Jan 2011
    Location
    Dystopia
    Posts
    3,755
    Rep Power
    7

    Default

    Yeah no problems. My consulting fees are $1000 an hour.

  3. #3
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

    Default

    Just a cursory glance - and, hence only worth 2c:

    Remove all the static declarations (excepting the static void main()) and deal with the helpful compiler messages.
    Remove the //-comments but add javadoc comments to say in a sentence (maybe two) what each method does.

  4. #4
    cedron is offline Member
    Join Date
    Jun 2011
    Posts
    11
    Rep Power
    0

    Default

    Quote Originally Posted by Junky View Post
    Yeah no problems. My consulting fees are $1000 an hour.
    Classy reply

  5. #5
    Zeramat is offline Member
    Join Date
    Jun 2011
    Posts
    5
    Rep Power
    0

    Default

    Junky, I wish I had the financial resources to pay that hourly rate for reviews.

    pbrokway2, Thank you for your response.

    To the first point, my intention was to isolate I/O routines to a single class. However without static declarations I found that I had to keep creating new instances of the Interface class, which is counter to the purpose of that class. Is there a better way to accomplish this same goal?

    To the second point, I will skip ahead in my book and read the javadoc chapter.

  6. #6
    sunde887's Avatar
    sunde887 is offline Moderator
    Join Date
    Jan 2011
    Location
    Richmond, Virginia
    Posts
    3,069
    Blog Entries
    3
    Rep Power
    8

    Default

    I could be wrong, but consider a singleton for the io routine class. Something like,
    Java Code:
    public class IORoutines{
      public static final INSTANCE = new IORoutines(...);
      private IORoutines(...){ 
        //initialization if needed
      }
      //IO methods
    }
    Then you can call methods like this

    Java Code:
    IORoutines.INSTANCE.someIOMethod();
    According to effective java, it's safer to use enums, but I can't remember the exact implementation off the top of my head.

  7. #7
    Junky's Avatar
    Junky is offline Grand Poobah
    Join Date
    Jan 2011
    Location
    Dystopia
    Posts
    3,755
    Rep Power
    7

    Default

    My post was tongue in cheek. The people here offer their time free of charge to help people with specific programming problems. The chances anyone will wade through elebenty gazillion lines of code just for kicks is very low.

  8. #8
    sunde887's Avatar
    sunde887 is offline Moderator
    Join Date
    Jan 2011
    Location
    Richmond, Virginia
    Posts
    3,069
    Blog Entries
    3
    Rep Power
    8

    Default

    By the way, I believe the enum type would look like this
    Java Code:
    public enum IORoutines{
      INSTANCE;
      //methods
    }
    And can be called identically to the way above.

  9. #9
    yellowledbet is offline Senior Member
    Join Date
    Feb 2011
    Location
    Georgia, USA
    Posts
    122
    Rep Power
    0

    Default

    Quote Originally Posted by sunde887 View Post
    I could be wrong, but consider a singleton for the io routine class. Something like,
    Java Code:
    public class IORoutines{
      public static final INSTANCE = new IORoutines(...);
      private IORoutines(...){ 
        //initialization if needed
      }
      //IO methods
    }
    Then you can call methods like this

    Java Code:
    IORoutines.INSTANCE.someIOMethod();
    I wrote a singleton class for a serializable data class for a small project recently. I am not sure whether this is a good or bad practice, but it definitely simplified my code. I will see if it stands the test of time.

  10. #10
    Zeramat is offline Member
    Join Date
    Jun 2011
    Posts
    5
    Rep Power
    0

    Default

    sunde887, Thank you for your responses. This does seem a bit cleaner over all.

    Junky, Believe it or not, I realized your post was tongue in cheek. Apparently elebenty gazillion equals 557. I picked this forum for asking for a review, as there are several posts where people include significant chunks of code, and have gotten good answers. I realize asking someone to take the time to wade through my chicken scratches is a big request. To be honest I would have been / am happy with a single useful reply.

Similar Threads

  1. Requesting a review of my LoginBox!
    By aadem in forum New To Java
    Replies: 4
    Last Post: 03-16-2011, 09:01 PM
  2. Receiving code review
    By sunde887 in forum New To Java
    Replies: 0
    Last Post: 03-08-2011, 05:31 AM
  3. please review the following code
    By ajbj in forum New To Java
    Replies: 3
    Last Post: 08-25-2009, 08:37 AM
  4. Please Review My Code (Long Integer Addition)
    By Saradus in forum New To Java
    Replies: 12
    Last Post: 07-05-2009, 01:01 PM
  5. [SOLVED] Code review
    By saeedsubedar in forum Advanced Java
    Replies: 14
    Last Post: 06-25-2008, 05:25 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
  •