Program Critique

Critique of my first java application

  • (2 Pages)
  • +
  • 1
  • 2

21 Replies - 2121 Views - Last Post: 01 September 2010 - 05:42 PM Rate Topic: -----

#1 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Program Critique

Posted 30 August 2010 - 10:13 AM

This may be a repost (I do not know where the original went).

I'd like a critique of my first java application.

It's a rock paper scissors game but it's not a game yet. It's just a structure and a test set.

CLASS: Program
public class Program {

	public static void main(String[] args) {
		IAction player = null;
		IAction computer = null;
		
		// Test tie on ROCK
		player = ActionFactory.get(1);
		computer = ActionFactory.get(1);
		Comparer.compare(player, computer);
		
		// Test tie on PAPER
		player = ActionFactory.get(2);
		computer = ActionFactory.get(2);
		Comparer.compare(player, computer);
		
		// Test tie on SCISORS
		player = ActionFactory.get(3);
		computer = ActionFactory.get(3);
		Comparer.compare(player, computer);
		
		// Player beats computer ROCK beats SCISORS
		player = ActionFactory.get(1);
		computer = ActionFactory.get(3);
		Comparer.compare(player, computer);
		
		// Player beats computer PAPER beats ROCK
		player = ActionFactory.get(2);
		computer = ActionFactory.get(1);
		Comparer.compare(player, computer);
		
		// Player beats computer SCISORS beats PAPER
		player = ActionFactory.get(3);
		computer = ActionFactory.get(2);
		Comparer.compare(player, computer);
		
		// Computer beats player ROCK beats SCISORS
		player = ActionFactory.get(3);
		computer = ActionFactory.get(1);
		Comparer.compare(player, computer);
		
		// Computer beats player PAPER beats ROCK
		player = ActionFactory.get(1);
		computer = ActionFactory.get(2);
		Comparer.compare(player, computer);
		
		// Computer beats player SCISORS beats PAPER
		player = ActionFactory.get(2);
		computer = ActionFactory.get(3);
		Comparer.compare(player, computer);
	}
}



INTERFACE: IAction
public interface IAction {
	public String getName();
	
	public IAction getBeats();
}



CLASS: Rock, Paper, Scissors
public class Rock implements IAction {
	
	public String getName()
	{
		return "Rock";
	}
	
	public IAction getBeats()
	{
		return new Scisors();
	}
}

public class Paper implements IAction {

	public String getName()
	{
		return "Paper";
	}
	
	public IAction getBeats()
	{
		return new Rock();
	}
}

public class Scisors implements IAction {

	public String getName()
	{
		return "Scisors";
	}
	
	public IAction getBeats()
	{
		return new Paper();
	}
}



CLASS: ActionFactory

public class ActionFactory {
	public static IAction get(int action)
	{
		switch (action) {
		case 1:
			return new Rock();
		case 2:
			return new Paper();
		case 3:
			return new Scisors();
		default:
			return null;
		
		}	
	}
}



CLASS: Comparer
public class Comparer {

	public static void compare(IAction player, IAction computer)
	{
		if (player.getName() == computer.getName())
			System.out.println(player.getName() + " ties " + computer.getName());
		else
		{
			IAction toWin = player.getBeats();
			if (computer.getName() == toWin.getName())
				System.out.println(player.getName() + " beats " + computer.getName());
			else
				System.out.println(player.getName() + " loses to " + computer.getName());
		}
	}
}



Finally, output:

Quote

Rock ties Rock
Paper ties Paper
Scisors ties Scisors
Rock beats Scisors
Paper beats Rock
Scisors beats Paper
Scisors loses to Rock
Rock loses to Paper
Paper loses to Scisors


Anything you can comment on would be great.

I noticed that you cannot switch statement a string. Is there a best practice on this to achieve close to the same thing as a string compare? I don't like these numbers. I suppose I can use an enum but I'm not sure how to structure it.

Thanks.

Oh and attached is the source code if you want to look at the whole project.

EDIT:
Oh and I know I was too dumb to recognize how to spell scissors... save it.

Attached File(s)


This post has been edited by MentalFloss: 30 August 2010 - 10:17 AM


Is This A Good Question/Topic? 0
  • +

Replies To: Program Critique

#2 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 30 August 2010 - 10:49 AM

OK. I modified it to use enums now.

ENUM: Actions
public enum Actions {
	ROCK,
	PAPER,
	SCISSORS
}



CLASS: Program
public class Program {

	public static void main(String[] args) {
		IAction player = null;
		IAction computer = null;
		
		// Test tie on ROCK
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Test tie on PAPER
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Test tie on SCISORS
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
		
		// Player beats computer ROCK beats SCISORS
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
		
		// Player beats computer PAPER beats ROCK
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Player beats computer SCISORS beats PAPER
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Computer beats player ROCK beats SCISORS
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Computer beats player PAPER beats ROCK
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Computer beats player SCISORS beats PAPER
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
	}
}



CLASS: ActionFactory
public class ActionFactory {
	public static IAction get(Actions action)
	{
		switch (action) {
		case ROCK:
			return new Rock();
		case PAPER:
			return new Paper();
		case SCISSORS:
			return new Scissors();
		default:
			return null;
		
		}	
	}
}



OUTPUT:

Quote

Rock ties Rock
Paper ties Paper
Scissors ties Scissors
Rock beats Scissors
Paper beats Rock
Scissors beats Paper
Scissors loses to Rock
Rock loses to Paper
Paper loses to Scissors


Attached is updated project.

Moved everything out of program and added a TestGame class to be responsible for a test run:

CLASS: TestGame
public class TestGame {
	
	public void play()
	{
		IAction player = null;
		IAction computer = null;
		
		// Test tie on ROCK
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Test tie on PAPER
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Test tie on SCISORS
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
		
		// Player beats computer ROCK beats SCISORS
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
		
		// Player beats computer PAPER beats ROCK
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Player beats computer SCISORS beats PAPER
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Computer beats player ROCK beats SCISORS
		player = ActionFactory.get(Actions.SCISSORS);
		computer = ActionFactory.get(Actions.ROCK);
		Comparer.compare(player, computer);
		
		// Computer beats player PAPER beats ROCK
		player = ActionFactory.get(Actions.ROCK);
		computer = ActionFactory.get(Actions.PAPER);
		Comparer.compare(player, computer);
		
		// Computer beats player SCISORS beats PAPER
		player = ActionFactory.get(Actions.PAPER);
		computer = ActionFactory.get(Actions.SCISSORS);
		Comparer.compare(player, computer);
	}
}



CLASS: Program
public class Program {

	public static void main(String[] args) {
		
		TestGame game = new TestGame();
		game.play();
	}
}



OUTPUT:

Quote

Rock ties Rock
Paper ties Paper
Scissors ties Scissors
Rock beats Scissors
Paper beats Rock
Scissors beats Paper
Scissors loses to Rock
Rock loses to Paper
Paper loses to Scissors


Attached project again.

Attached File(s)


Was This Post Helpful? 0
  • +
  • -

#3 Luckless  Icon User is offline

  • </luck>
  • member icon

Reputation: 292
  • View blog
  • Posts: 1,146
  • Joined: 31-August 09

Re: Program Critique

Posted 30 August 2010 - 11:02 AM

good deal. Looks real clean to me. My only suggestion would be to use the Random() class to create a random number between 1 and 3... and so on.

Seriously though, good stuff for a first app, you must be from C++ or something similar. Program on!
Was This Post Helpful? 1
  • +
  • -

#4 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 30 August 2010 - 11:15 AM

View PostLuckless, on 30 August 2010 - 10:02 AM, said:

good deal. Looks real clean to me. My only suggestion would be to use the Random() class to create a random number between 1 and 3... and so on.


I'm trying to figure out how I can actually input into console for player choice. I'll use random for computer of course but I can't even make it an actual game until I can add my own input.

View PostLuckless, on 30 August 2010 - 10:02 AM, said:

Seriously though, good stuff for a first app, you must be from C++ or something similar. Program on!


C#
Was This Post Helpful? 0
  • +
  • -

#5 javadork  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 32
  • View blog
  • Posts: 135
  • Joined: 21-August 10

Re: Program Critique

Posted 30 August 2010 - 11:31 AM

Excellent first crack at Java. To make the learning process more challenging, modify TestGame to use more formal unit testing. Take a look at JUnit.
Was This Post Helpful? 1
  • +
  • -

#6 macosxnerd101  Icon User is offline

  • Self-Trained Economist
  • member icon




Reputation: 10176
  • View blog
  • Posts: 37,571
  • Joined: 27-December 08

Re: Program Critique

Posted 30 August 2010 - 11:46 AM

The whole point of enums is that you don't have to create new Objects each time. Enums are classes with fixed instances, which makes comparing them with the == operator safe, as they have the same addresses in memory or not. So ROCK == ROCK always. So you can eliminate ActionFactory all together. Plus, you can define a Comparator<E> interface for your enum to compare two variables of your enum type. If you are going to define the comparison, might as well use the Comparator interface.

As for console input, take a look at the Scanner class or BufferedReader class. If you pass them System.in (the InputStream for the console), you can parse console input.
Was This Post Helpful? 1
  • +
  • -

#7 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 30 August 2010 - 11:54 AM

View Postmacosxnerd101, on 30 August 2010 - 10:46 AM, said:

The whole point of enums is that you don't have to create new Objects each time. Enums are classes with fixed instances, which makes comparing them with the == operator safe, as they have the same addresses in memory or not. So ROCK == ROCK always. So you can eliminate ActionFactory all together. Plus, you can define a Comparator<E> interface for your enum to compare two variables of your enum type. If you are going to define the comparison, might as well use the Comparator interface.

As for console input, take a look at the Scanner class or BufferedReader class. If you pass them System.in (the InputStream for the console), you can parse console input.


As for my program, yeah it will all change as I learn how to get input.

Will it pop up like a black box like with C#? It normally looks like a separate window but with eclipse, it's just a docked pane that appears to lack any form of input abilities.

Also, how do I "take a look at the Scanner class"? Is it all already included and I just have to import it or does it require a download of it from somewhere?

I'm sorry. You get a chance to see me being a total complete newbie at something for once :P
Was This Post Helpful? 0
  • +
  • -

#8 macosxnerd101  Icon User is offline

  • Self-Trained Economist
  • member icon




Reputation: 10176
  • View blog
  • Posts: 37,571
  • Joined: 27-December 08

Re: Program Critique

Posted 30 August 2010 - 11:58 AM

If you want a popup window, you can use the JOptionPane class. Import it from the javax.swing package. For Scanner, it is the docked window on your IDE (or command line if you are using that). You import Scanner from java.util.

As for the classes, if you Google Java 6 <classname here>, you will get links to Java's awesome documentation that covers everything you could ever want to know about the class, including a description of what it does, and a very organized display of the accessible variables, the methods, and the inherited attributes and methods. Both of these classes come standard with the JDK. :)
Was This Post Helpful? 1
  • +
  • -

#9 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 30 August 2010 - 11:59 AM

I found a tutorial site... lol

http://www.java-made...-beginners.html

EDIT:

And what I need: http://www.java-made...ssing-game.html

(User input)

This post has been edited by MentalFloss: 30 August 2010 - 12:01 PM

Was This Post Helpful? 0
  • +
  • -

#10 Dogstopper  Icon User is offline

  • The Ninjaducky
  • member icon



Reputation: 2857
  • View blog
  • Posts: 10,960
  • Joined: 15-July 08

Re: Program Critique

Posted 30 August 2010 - 01:59 PM

You can find the awesome Scanner docs here:
http://download-llnw...il/Scanner.html

And awesome free Java tutorials here:
http://download.orac...avase/tutorial/

Comparators are so cool. I highly suggest that you learn them:
http://lkamal.blogsp...comparable.html
Was This Post Helpful? 1
  • +
  • -

#11 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 31 August 2010 - 11:43 AM

Here's my attempt at doing that guessing game tutorial. You can critique that instead.

(Figured it'd be clutter to add a new thread)

import java.util.Random;
import java.util.Scanner;

public class Program {
	
	static Scanner input = new Scanner(System.in);
	static int low = 1;
	static int high = 1000;
	
	public static void main(String[] args) {
		
		Random rand = new Random();
		int numberToGuess = rand.nextInt(high);
		int numberOfTries = 0;
		int guess = -1;
		
		while (guess != numberToGuess) {
			guess = getNextGuess();
			printResult(guess, numberToGuess);
			numberOfTries++;
		}
		
		System.out.println("It took " + numberOfTries + " tries to find it.");
	}
	
	private static int getNextGuess()
	{
		int guess;
		System.out.println("Guess a number between " + low + " and " + high + ": ");
		guess = input.nextInt();
		return guess;
	}
	
	private static void printResult(int guess, int numberToGuess)
	{
		if (guess >numberToGuess)
		{
			System.out.println("Too high!");
			high = guess;
		}
		else if (guess < numberToGuess)
		{
			System.out.println("Too low!");
			low = guess;
		}
		else
			System.out.println("You Win!");
	}

}



Sample run:

Quote

Guess a number between 1 and 1000:
500
Too high!
Guess a number between 1 and 500:
200
Too high!
Guess a number between 1 and 200:
50
Too high!
Guess a number between 1 and 50:
10
Too low!
Guess a number between 10 and 50:
20
Too low!
Guess a number between 20 and 50:
30
Too low!
Guess a number between 30 and 50:
40
Too high!
Guess a number between 30 and 40:
35
You Win!
It took 8 tries to find it.


It's been a while since I've written anything as goofy as a game like this so I'm guessing I have a few screw ups somewhere. Any help is appreciated.
Was This Post Helpful? 0
  • +
  • -

#12 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 31 August 2010 - 12:54 PM

Back to rock paper scissors (I figure I'll just throw them both up into same thread and you can critique them as you see fit)

I removed the class declarations of Rock,Paper,Scissors. I also removed the interface IAction. I changed the factory to be an enum mapper.

CLASS: Program
public class Program {

	public static void main(String[] args) {
		TestGame game = new TestGame();
		game.play();
	}
}



CLASS: TestGame
public class TestGame {
	
	public void play()
	{
		Actions player = null;
		Actions computer = null;
		Results outcome = null;
		// Test tie on ROCK
		player = ActionMapper.get(1);
		computer = ActionMapper.get(1);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Test tie on PAPER
		player = ActionMapper.get(2);
		computer = ActionMapper.get(2);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Test tie on SCISORS
		player = ActionMapper.get(3);
		computer = ActionMapper.get(3);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Player beats computer ROCK beats SCISORS
		player = ActionMapper.get(1);
		computer = ActionMapper.get(3);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Player beats computer PAPER beats ROCK
		player = ActionMapper.get(2);
		computer = ActionMapper.get(1);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Player beats computer SCISORS beats PAPER
		player = ActionMapper.get(3);
		computer = ActionMapper.get(2);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Computer beats player ROCK beats SCISORS
		player = ActionMapper.get(3);
		computer = ActionMapper.get(1);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Computer beats player PAPER beats ROCK
		player = ActionMapper.get(1);
		computer = ActionMapper.get(2);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
		
		// Computer beats player SCISORS beats PAPER
		player = ActionMapper.get(2);
		computer = ActionMapper.get(3);
		outcome = Comparer.compare(player, computer);
		Display.printResult(player, computer, outcome);
	}
}



ENUM: Actions
public enum Actions {
	ROCK,
	PAPER,
	SCISSORS
}



ENUM: Results
public enum Results {
	PLAYER,
	COMPUTER,
	TIE
}



CLASS: ActionMapper
public class ActionMapper {
	public static Actions get(int action)
	{
		switch (action) {
		case 1:
			return Actions.ROCK;
		case 2:
			return Actions.PAPER;
		case 3:
			return Actions.SCISSORS;
		default:
			return null;
		
		}	
	}
}



CLASS: Comparer
public class Comparer {

	public static Results compare(Actions player, Actions computer) {
		
		Results outcome = Results.TIE;
		
		switch (player)
		{
		case ROCK:
			if (computer == Actions.ROCK) {
				outcome = Results.TIE;
			} else if (computer == Actions.PAPER) {
				outcome = Results.COMPUTER;
			} else if (computer == Actions.SCISSORS) {
				outcome = Results.PLAYER;
			}
			break;
			
		case PAPER:
			if (computer == Actions.ROCK) {
				outcome = Results.PLAYER;
			} else if (computer == Actions.PAPER) {
				outcome = Results.TIE;
			} else if (computer == Actions.SCISSORS) {
				outcome = Results.COMPUTER;
			}
			break;
			
		case SCISSORS:
			if (computer == Actions.ROCK) {
				outcome = Results.COMPUTER;
			} else if (computer == Actions.PAPER) {
				outcome = Results.PLAYER;
			} else if (computer == Actions.SCISSORS) {
				outcome = Results.TIE;
			}
			break;
		}
		
		return outcome;
	}
}



CLASS: Display
public class Display {

	public static void printResult(Actions player, Actions computer, Results outcome) {
		
		switch (outcome)
		{
		case PLAYER:
			System.out.println(player.toString() + " beats " + computer.toString());
			break;
		case COMPUTER:
			System.out.println(player.toString() + " loses to " + computer.toString());
			break;
		case TIE:
			System.out.println(player.toString() + " ties " + computer.toString());
			break;
		}
	}
	
}



OUTPUT

Quote

ROCK ties ROCK
PAPER ties PAPER
SCISSORS ties SCISSORS
ROCK beats SCISSORS
PAPER beats ROCK
SCISSORS beats PAPER
SCISSORS loses to ROCK
ROCK loses to PAPER
PAPER loses to SCISSORS


A few things I'm still working on:
  • Getting JUnit working
  • Implementing Comparator interface
  • Turning it into an actual game


Thanks for any feedback you feel compelled to share with me.

Attached is source code.


EDIT:

OK. I made a game skeleton:

CLASS: Program
public class Program {

	public static void main(String[] args) {
		TestGame game = new TestGame();
		game.play();
		
		// Play a real game:
		System.out.println();
		System.out.println();
		
		Game realGame = new Game();
		realGame.play();
	}
}



CLASS: Game
import java.util.Scanner;
import java.util.Random;

public class Game {

	static Scanner input = new Scanner(System.in);
	
	public void play()
	{
		System.out.println("Pick One: Rock=1, Paper=2, Scissors=3:");
		int playerChoice = input.nextInt();
		
		Random random = new Random();
		int computerChoice = random.nextInt(3)+1; // 1, 2, 3 (random starts at zero)
		
		Actions player = ActionMapper.get(playerChoice);
		Actions computer = ActionMapper.get(computerChoice);
		
		Results outcome = Comparer.compare(player, computer);
		
		Display.printResult(player, computer, outcome);
	}
}



Any thoughts? Is there a better way to do random? Perhaps adding 1 to it is pointless? However, it said 0 is inclusive and the value is exclusive.

So, nextInt(3) is set 0, 1, 2. and nextInt(3)+1 is set 1, 2, 3.

EDIT:

OK. Game now keeps going and keeps score:

CLASS: Game
import java.util.Scanner;
import java.util.Random;

public class Game {

	static Scanner input = new Scanner(System.in);
	
	public void play()
	{
		boolean keepPlaying = true;
		int wins = 0;
		int losses = 0;
		int ties = 0;
		
		while (keepPlaying)
		{
			System.out.println("Pick One: Rock=1, Paper=2, Scissors=3:");
			int playerChoice = input.nextInt();
			
			Random random = new Random();
			int computerChoice = random.nextInt(3)+1; // 1, 2, 3 (random starts at zero)
			
			Actions player = ActionMapper.get(playerChoice);
			Actions computer = ActionMapper.get(computerChoice);
			
			Results outcome = Comparer.compare(player, computer);
			
			switch (outcome)
			{
			case PLAYER:
				wins++;
				break;
			case COMPUTER:
				losses++;
				break;
			case TIE:
				ties++;
				break;
			}
			
			Display.printResult(player, computer, outcome);
			
			System.out.println("Play again?: Yes=1, No=2");
			int playAgain = input.nextInt();
			
			keepPlaying = playAgain == 1;
		}
		
		System.out.println("Result: Wins=" + wins + ", Losses=" + losses + ", Ties=" + ties);
	}
}



Any critiques?

Attached File(s)


This post has been edited by MentalFloss: 31 August 2010 - 01:25 PM

Was This Post Helpful? 0
  • +
  • -

#13 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 31 August 2010 - 01:32 PM

Here's the updated game:

CLASS: Game
import java.util.Scanner;
import java.util.Random;

public class Game {

	static Scanner input = new Scanner(System.in);
	
	private int wins = 0;
	private int losses = 0;
	private int ties = 0;
	
	public void play()
	{
		boolean keepPlaying = true;

		while (keepPlaying)
		{
			System.out.println("Pick One: Rock=1, Paper=2, Scissors=3:");
			int playerChoice = input.nextInt();
			
			Random random = new Random();
			int computerChoice = random.nextInt(3)+1; // 1, 2, 3 (random starts at zero)
			
			Actions player = ActionMapper.get(playerChoice);
			Actions computer = ActionMapper.get(computerChoice);
			
			Results outcome = Comparer.compare(player, computer);

			Display.printResult(player, computer, outcome);
			
			updateScores(outcome);
			
			keepPlaying = playAnotherGame();
		}
		printScoreboard();
	}
	
	private void updateScores(Results outcome) {
		switch (outcome)
		{
		case PLAYER:
			wins++;
			break;
		case COMPUTER:
			losses++;
			break;
		case TIE:
			ties++;
			break;
		}
	}
	
	private boolean playAnotherGame(){
		System.out.println("Play again?: Yes=1, No=2");
		int playAgain = input.nextInt();
		
		return (playAgain == 1);
	}
	
	private void printScoreboard(){
		System.out.println("Result: Wins=" + wins + ", Losses=" + losses + ", Ties=" + ties);
	}
}


Was This Post Helpful? 0
  • +
  • -

#14 KYA  Icon User is offline

  • g++ jameson.cpp -o beverage
  • member icon

Reputation: 3088
  • View blog
  • Posts: 19,136
  • Joined: 14-September 07

Re: Program Critique

Posted 31 August 2010 - 01:42 PM

You could get rid of the ActionMapper entirely and put the "action" inside of the Actions enum. Currently, as is, using it as an enum is pointless.

I would throw it all into one enum:

import java.lang.*;
import java.io.*;

public enum Actions{
	//keeping with the numerical equivalents already present
	ROCK(1, "Rock"),
	PAPER(2, "Paper"),
	SCISSORS(3, "Scissors");
	
	private int status;
	private String name;
	
	//private enum
	enum Outcome{
		TIE ("It was a Tie"),
		PLAYER ("Player won"),
		COMPUTER("Computer Won");
		
		String result;
		
		private Outcome(String r){
			result = r;
		}
		public String getResult()	{return result;}
	}
	
	private Actions(int value, String n){
		status = value;
		name = n;
	}
	
	public String getName()	{return name;}
	
	public static Actions getValue(int v){
		switch(v){
			case 1:
				return Actions.ROCK;
			case 2:
				return Actions.PAPER;
			case 3:
				return Actions.SCISSORS;
			default:
				return Actions.ROCK;
		}
	}
	
	public Outcome compare(Actions rhs){
		switch(status){
			case 1:
				if (rhs == Actions.PAPER) return Outcome.COMPUTER;
				else if (rhs == Actions.ROCK) return Outcome.TIE;
				else return Outcome.PLAYER;
			case 2:
				if (rhs == Actions.PAPER) return Outcome.TIE;
				else if (rhs == Actions.ROCK) return Outcome.PLAYER;
				else return Outcome.COMPUTER;
			case 3:
				if (rhs == Actions.PAPER) return Outcome.PLAYER;
				else if (rhs == Actions.ROCK) return Outcome.COMPUTER;
				else return Outcome.TIE;
			default:
					return Outcome.TIE;
		}
	}
}



Example usage:

import java.lang.*;
import java.io.*;
import java.util.*;

class Test{
	public static void main(String[] args){
		Random r = new Random();
		Actions[] playerHand = new Actions[3];
		Actions[] computerHand = new Actions[3];
		
		//generate rnadomly for illustrative purposes
		for(int i = 0; i < 3; i++){
			playerHand[i] = Actions.getValue(r.nextInt() % 3 + 1);
			computerHand[i] = Actions.getValue(r.nextInt() % 3 + 1);
		}
		
		//show the "hands" debug purposes
		System.out.println("Player's hand:");
		for(int i = 0; i < 3; i++){
			System.out.print(playerHand[i].getName() + " ");
		}
		System.out.println();
		System.out.println("Computer's hand:");
		for(int i = 0; i < 3; i++){
			System.out.print(computerHand[i].getName() + " ");
		}
		
		System.out.println();
		System.out.println();
		
		//compare and fight
		for(int i = 0; i < 3; i++){
			System.out.print("Player: " + playerHand[i].getName() + " versus " + "Computer: " + 
				computerHand[i].getName() + "\t");
			System.out.println(playerHand[i].compare(computerHand[i]).getResult());
			System.out.println();
		}
	}
}




Output of a few runs:

Quote

Player's hand:
Rock Rock Rock
Computer's hand:
Paper Rock Rock

Player: Rock versus Computer: Paper Computer Won

Player: Rock versus Computer: Rock It was a Tie

Player: Rock versus Computer: Rock It was a Tie


Player's hand:
Rock Rock Rock
Computer's hand:
Rock Scissors Rock

Player: Rock versus Computer: Rock It was a Tie

Player: Rock versus Computer: Scissors Player won

Player: Rock versus Computer: Rock It was a Tie


Player's hand:
Scissors Scissors Paper
Computer's hand:
Paper Rock Rock

Player: Scissors versus Computer: Paper Player won

Player: Scissors versus Computer: Rock Computer Won

Player: Paper versus Computer: Rock Player won

Was This Post Helpful? 1
  • +
  • -

#15 MentalFloss  Icon User is offline

  • "ADDICTED"[2:5]
  • member icon

Reputation: 525
  • View blog
  • Posts: 1,397
  • Joined: 02-September 09

Re: Program Critique

Posted 31 August 2010 - 01:49 PM

Thank you KYA. I clearly have a lot of new things to learn about from that post.
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2