• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Tim Cooke
  • paul wheaton
  • Jeanne Boyarsky
  • Ron McLeod
Sheriffs:
  • Paul Clapham
  • Liutauras Vilda
  • Devaka Cooray
Saloon Keepers:
  • Tim Holloway
  • Roland Mueller
Bartenders:

Scissors-rock-paper game looking for better way to write the program

 
Greenhorn
Posts: 7
Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The task is to write a program that plays the scissors-rock-paper game. The program should let the user continuously play until either the user or the computer wins more than two times (I assume in a row).
I'm confused about the while loop in the code I wrote:



I've tried to use



instead of how it is in the code, but that doesn't seem to worked as I've expected. It keeps looping no matter how many times user or respectively computer won.
Any suggestions how to write the program better are greatly appreciated.
 
Greenhorn
Posts: 25
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I just ran the exact code you had and it worked fine. It stopped after 2 wins. Dunno what the problem is :] Generally its better practice I think to use a boolean flag for the while loops such as


But it worked fine for me the way you had it.
 
Marshal
Posts: 80267
430
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Even though Winston Gutkowski will disagree, you should never use == false or == true or similar. It’s not while (b == false) ..., but while (!b) ...
You can get rid of the bang operator (“!”) like thisThat saves you using the bang operator, which some people find hard to understand. It also gets you out of using if (...) b = true;, which is not as bad as the if-else form shown here, but it’s still neater in the form I used. [There are some instances where the logic does require if (...) b = true; or if (...) b = false;] You should also avoid the literal 2 if possible; declare it as final somewhereThat way you know it is the winning margin, and you can also change to 3 up easily.

*   *   *   *   *   *   *

It always worries me when I see a main method with more than one statement in. It suggests there is no object-orientation in the design of the app.
I think you need an enum with ROCK PAPER SCISSORS as its elements, and an enum with WIN DRAW LOSE as its elements. You can have a Player class (the computer being a Player, too), with a choice method. Your Player can have a consecutiveWins field. Your RPS enum can have winning and losing fields, so when you call its play() method it can return WIN LOSE or DRAW. You can use a switch block with the three enum members as its cases. You can learn about enums in the Java Tutorials. You can use the ordinal() method of the enum to select your option, eg 0 = ROCK, 1 = PAPER, 2 = SCISSORS.
 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:Even though Winston Gutkowski will disagree, you should never use == false or == true or similar. It’s not while (b == false) ..., but while (!b) ...


Plainly I'm not alone in finding it clearer in some cases though.

@Venny: As Campbell says, I disagree with him on this point: x == false and !x are logically equivalent, and unless you think he might be vetting your code, I'd use whatever construct seems clearest to you. Just be sure to use '==' and not '='.
On the other hand, I totally agree with him about using enums: you can basically put practically the entire logic of the game into the RPS enum he suggested, and you'll save yourself a lot of validation code in the process.

Winston
 
Campbell Ritchie
Marshal
Posts: 80267
430
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
It’s my turn to wind you up now. As I showed, you can often rename the loop variable so you don’t need the bang operator.
If you are going to use == try false == b rather than b == false. I know that looks dreadfully like C code, but it means if you write = by mistake you will get a compiler error rather than logic errors at runtime.
 
Ranch Hand
Posts: 140
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm going to guess that the reason it's a big single method is because this is an introductory class, and maybe they haven't gotten to moving more into object-oriented code yet?

That said, if you're going for two wins in a row as being the deciding factor, then the logic looks correct. Clarity for the WHILE statement is going to vary, but my first "quick fix" instinct is to get rid of the IF at the end, and change the WHLE to:


And get rid of the WINNER variable entirely.

I haven't run your code - but I can't see why it doesn't work as-is... and Joseph Tulowiecki has run it without issue. No idea why it's failing to end... are your System.out.println statements actually showing you output where somebody has a score of 2, yet it keeps going?
 
Ranch Hand
Posts: 808
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Notice that the code in each switch case is almost verbatim the same; the only elements that vary are those specific to the throw. You can abstract those elements, and then the switch would very likely not be needed at all. Campbell's idea of using an enum will help you do that. To his excellent suggestions, let me add that you might consider giving the enum a String "display" field, which would be very useful in condensing all those repetitive system out statements.
 
Campbell Ritchie
Marshal
Posts: 80267
430
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Joe Vahabzadeh wrote:I'm going to guess that the reason it's a big single method is because this is an introductory class, and maybe they haven't gotten to moving more into object-oriented code yet? . . .

Or don’t realise how easy it is to get into bad (non-object-oriented) habits at the beginning, and how hard it is to break those bad habits.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dennis Deems wrote:To his excellent suggestions, let me add that you might consider giving the enum a String "display" field, which would be very useful in condensing all those repetitive system out statements.


Absolutely. I think I'd also add a method to convert a String to the enum. That way you can take whatever the user typed in and just convert it directly, validating it at the same time (eg, if the method returns null, the String supplied was invalid).

Winston
 
Venny Tank
Greenhorn
Posts: 7
Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thank you all of you guys for the suggestions!
I learn java (with no previous programming experience) on my own by reading the java's documentation, and a book following the exercises.

@Joe, @Campbell: Yes, I'm still on the chapter about loops, so don't know too much about methods yet, but will keep in mind and try not to develop bad programming habits.

I've tried avoiding the WINNER variable by using only:

but it keeps going to generate random number and asking for user input even if somebody has a score of 2 and more.
And my question now is: May I use a "while" loop with two conditional statements like the code above?

I'll read further about enums and using methods.

Thank you all for your help
 
Sheriff
Posts: 28370
99
Eclipse IDE Firefox Browser MySQL Database
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:@Venny: As Campbell says, I disagree with him on this point: x == false and !x are logically equivalent, and unless you think he might be vetting your code, I'd use whatever construct seems clearest to you. Just be sure to use '==' and not '='.



Staying off topic for a bit longer: if you find that "! x" is unclear, then there's a good chance that's because you chose an opaque variable name like "x". Now if you had chosen a descriptive variable name like "finished" then you'll probably find that you can tell what "if (! finished) ..." means. And you'll find that assignments like "finished = true" have an obvious meaning too.
 
dennis deems
Ranch Hand
Posts: 808
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Venny Tank wrote:And my question now is: May I use a "while" loop with two conditional statements like the code above?


Absolutely. Any expression which evaluates to true or false, however complex, may be used as the exit condition of a while loop.
 
Bartender
Posts: 1051
5
Hibernate Eclipse IDE Chrome
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Venny

Once you have refactored your code with some of the excellent suggestions provided, perhaps consider additional changes to introduce classes such as Player and Game, thus making your code more extensible.
 
Joseph Tulowiecki
Greenhorn
Posts: 25
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You would have to use while ( (userWins < 2) && (compWins <2)){

instead of ||

when you use " || " your saying .. Continue looping as long as either the user has less than 2 wins or the comp has less than 2 wins. This will never stop looping because it is Always true. If the computer has two twins then user has 0 wins and vice versa so one of them will always have less than 2. Therefore the loop will never end. If you use && which means "and" it will give you the results your looking for.
 
Venny Tank
Greenhorn
Posts: 7
Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks everyone

Joseph Tulowiecki wrote:You would have to use while ( (userWins < 2) && (compWins <2)){

instead of ||

when you use " || " your saying .. Continue looping as long as either the user has less than 2 wins or the comp has less than 2 wins. This will never stop looping because it is Always true. If the computer has two twins then user has 0 wins and vice versa so one of them will always have less than 2. Therefore the loop will never end. If you use && which means "and" it will give you the results your looking for.



Very well explained why the code:

never stops looping in my code, now I understand

Thanks again
 
Rototillers convert rich soil into dirt. Please note that this tiny ad is not a rototiller:
Smokeless wood heat with a rocket mass heater
https://woodheat.net
reply
    Bookmark Topic Watch Topic
  • New Topic