(11-04-2013, 12:52 AM)Frooxius Wrote: Just a quick suggestion: Try multiplying the random change by 3, so you don't have to compare it against fractional numbers. Like this:
Code:
public static String pcChoice() {
double random = Math.random()*3;
String choicePc = null;
if (random <= 1) {
choicePc = "rock";
}
if ((random > 1) && (random <= 2)) {
choicePc = "paper";
}
if (random > 3) {
choicePc = "scissors";
}
return choicePc;
}
Using Math.random() is a bad idea in general if all you need is a choice of three, which is an integer range.
Math.random() returns a double.
Random.nextInt(n) returns an int and should be prefered as it is less biased. Before I repeat any explanation I refer to this discussion about the difference:
http://stackoverflow.com/questions/73862...nextintint
@"blackeagle"
That's exactly what I wanted with the methods.
I also see you have put a game loop into it too.
Now let's go down to the details. Some of these where already mentioned by @Frooxius and @"ArkPhaze".
You should really have a look into enums and see how you can improve your program with your newly gained knowledge.
Look here for more:
http://howtodoinjava.com/2012/12/07/guid...m-in-java/
http://docs.oracle.com/javase/tutorial/j.../enum.html
About the random choice for the PC. Java has a Random class that provides several methods to get random values. See at the documentation here:
http://docs.oracle.com/javase/7/docs/api...andom.html
Try to get used to reading the documentation if you have any questions regarding how and when to use certain classes or methods.
If you have problems understanding it right now, don't worry. You will get used to it. Just look a bit for methods that seem to suit your needs. (I actually wanted to go there by letting you choose the right one, but I already mentioned it to Frooxius above, so, well, let's go on) So find the nextInt(n) method, read its explanation and try to use it for your program.
If you have any problems, come back to me. It is very important that you learn to help yourself, which is why I prefer that you try first and possibly come back with questions.
We will look at the pc choice method afterwards to improve it further.
Code:
while ((!choice.equalsIgnoreCase("rock")) && (!choice.equalsIgnoreCase("paper")) && (!choice.equalsIgnoreCase("scissors")))
If you have a relatively complicated conditional expression like this, prefer to extract it as a method too.
In this case you can name it i.e. isValidChoice and pass the user's choice into it.
The outline:
Code:
private boolean isValidChoice(String choice) {
//your code
}
And you would use it like this:
Code:
while(isValidChoice(choice)) {
The biggest advantage here is the better readability.
chooseWinner is a relatively repetitive method implementation we can shorten a lot once you read about enums.
Code:
} while (playAgain.equalsIgnoreCase("y"));
if (playAgain.equals("n")) {
System.out.println("thx for playing");
}
You are using equals and equalsIgnoreCase incosistently. One time the user is allowed to press Y and the other time he isn't.
Instead of doing this all the time and creating possible bugs, make playAgain a boolean. Check once if the userinput equalsIgnoreCase("y") and then set the correct boolean value to playAgain. This way you also avoid the need of consistency in the use of equals vs equalsIgnoreCase.
The last if statement will be superfluous then (after your corrections). After exiting the loop the if condition should always yield true, so just leave the if statement away and print your thanks string.