Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Rock,Paper,Scissors Code issue  RSS feed

 
stelios papamichael
Ranch Hand
Posts: 93
C++ Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello there!I am making a rock,paper,scissors game in eclipse which takes a random choice from the computer and an option from the user and compares them in a few nested if statements.My problem is that,whenever i run the application,although the two choices do 'work',the application never compares them and goes straight to the else {} part of the if statements.I do not know why this happens and sadly i did not manage to figure anything out.Here is my code so that you can try and help me :


Thanks in advance!
 
Campbell Ritchie
Marshal
Posts: 55681
161
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

I am afraid I haven't got the time to read your code properly, but I did see several incorrect instances of the == operator. That will not work.
 
Ganesh Patekar
Bartender
Posts: 696
23
Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to CodeRanch!
  • This is what happens when you use equality operator == OR equals() method to compare two Strings.
  • Output:
    strOne == strTwo: false
    strOne.equals(strTwo): true

  • Always use equals() method of String to compare contents of two Strings.
  • Don't write that much code in main method creates confusion and also hard to find logical errors, so better create a separate method you can name it getRockPaperScissors() or anything you feel more expressive.
  • Worth Reading click on -->  Main Is A Pain
  • Don't declare variables as static unless there is really specific reason for it. I don't think need to declare static here below, you can put this code in method.

  • Print message before taking input on console for user, asking enter your choice etc. here
  •  
    Campbell Ritchie
    Marshal
    Posts: 55681
    161
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Useful advice from GP
    Since you are using single words for the entry, I would suggest you use next() rather than nextLine() as the method used on the Scanner.
    I would suggest an alternative to your use of Math#random to choose the computer's move. Create a Random object and use its nextInt method and use the returned value to pick choices out of an arrayI would also suggest an alternative to your if statements: use a switch statement.

    There is a much more object‑oriented way to write that sort of game, using enumerated types, but that may take you more time to learn than you have at your disposal.
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:Welcome to CodeRanch!
  • This is what happens when you use equality operator == OR equals() method to compare two Strings.
  • Output:
    strOne == strTwo: false
    strOne.equals(strTwo): true

  • Always use equals() method of String to compare contents of two Strings.
  • Don't write that much code in main method creates confusion and also hard to find logical errors, so better create a separate method you can name it getRockPaperScissors() or anything you feel more expressive.
  • Worth Reading click on -->  Main Is A Pain
  • Don't declare variables as static unless there is really specific reason for it. I don't think need to declare static here below, you can put this code in method.

  • Print message before taking input on console for user, asking enter your choice etc. here


  • Thanks so so much m8!I had no idea there was an equals() method.From now on i will be using it!
    +I forgot i can create a seperate method and call it inside the main method.
    Also,the reason i declared my vars as static is because if i do not,i cannot use them inside the main method since that's a static one(at least that's what the error was telling me ).
    As for the message before the input,i was planning on doing so afterwards!

    Thanks again for your amazing help and kindness!
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Campbell Ritchie wrote:Useful advice from GP
    Since you are using single words for the entry, I would suggest you use next() rather than nextLine() as the method used on the Scanner.
    I would suggest an alternative to your use of Math#random to choose the computer's move. Create a Random object and use its nextInt method and use the returned value to pick choices out of an arrayI would also suggest an alternative to your if statements: use a switch statement.

    There is a much more object‑oriented way to write that sort of game, using enumerated types, but that may take you more time to learn than you have at your disposal.


    Thanks a lot for the advice dude!Appreciate your help.I was thinking of using a switch statement but wasn't quite sure on how to do it properly.As for the next() method of the scanner,i may be doing this since it seems to be more correct?!
    Thanks again ;) !
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:Welcome to CodeRanch!
  • This is what happens when you use equality operator == OR equals() method to compare two Strings.
  • Output:
    strOne == strTwo: false
    strOne.equals(strTwo): true

  • Always use equals() method of String to compare contents of two Strings.
  • Don't write that much code in main method creates confusion and also hard to find logical errors, so better create a separate method you can name it getRockPaperScissors() or anything you feel more expressive.
  • Worth Reading click on -->  Main Is A Pain
  • Don't declare variables as static unless there is really specific reason for it. I don't think need to declare static here below, you can put this code in method.

  • Print message before taking input on console for user, asking enter your choice etc. here


  • One more question.I really did improve this project a lot using the Main is a pain article(thanks so much for that one) but i had a question that i forgot to ask about in this thread.My scanner variables are outside of my run method(new method inside a new class)so that there are no errors.However,whenever i try to put system.out.println(); outside of the run method,it does not work and if this keeps up,i won't be able to log a message to the console so that the user knows when he has to type something.I guess this is a very basic question but what can i say :P !I am a begginer.(I looked it up and found some alternatives like importing some packages or using .writeline but these seem to be too advanced for my level).Thanks again in advance!
     
    Carey Brown
    Bartender
    Posts: 2985
    46
    Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Could you repost your code so that we can see the changes that you've made?
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    stelios papamichael wrote:the reason i declared my vars as static is because if i do not,i cannot use them inside the main method since that's a static one
    Yes you are right about non-static variables declared in class can't be accessed in static method But you can declare and initialize these two within the main method.(but create separate method for all that code as already mentioned).
    There is another method equalsIgnoreCase(String anotherString), you might understand what It does by it's name.This is how we should name the method so just by reading it we would come to know what it does.

    My scanner variables are outside of my run method(new method inside a new class)so that there are no errors.However,whenever i try to put system.out.println(); outside of the run method,it does not work and
    run method?   would be easier to help you, if you post that code.

    Campbell Ritchie wrote:Create a Random object and use its nextInt method
    Here OP is assigning value returned by Math.random() which returns a double value with a positive sign, greater than or equal to 0.0 and less than 1.0. OP is also not casting the value to int so I think there is no possible pitfall as you mentioned here, do you still suggest to use Random? If yes, can you please tell why?.
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Carey Brown wrote:Could you repost your code so that we can see the changes that you've made?


    Here is the MyGame class file :



    and this is my Logic class which handles the whole coding part :

     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:
    stelios papamichael wrote:the reason i declared my vars as static is because if i do not,i cannot use them inside the main method since that's a static one
    Yes you are right about non-static variables declared in class can't be accessed in static method But you can declare and initialize these two within the main method.(but create separate method for all that code as already mentioned).
    There is another method equalsIgnoreCase(String anotherString), you might understand what It does by it's name.This is how we should name the method so just by reading it we would come to know what it does.

    My scanner variables are outside of my run method(new method inside a new class)so that there are no errors.However,whenever i try to put system.out.println(); outside of the run method,it does not work and
    run method?   would be easier to help you, if you post that code.

    Campbell Ritchie wrote:Create a Random object and use its nextInt method
    Here OP is assigning value returned by Math.random() which returns a double value with a positive sign, greater than or equal to 0.0 and less than 1.0. OP is also not casting the value to int so I think there is no possible pitfall as you mentioned here, do you still suggest to use Random? If yes, can you please tell why?.


    I posted my new code in the quote above if you want to take a look at it.I hope it looks a lot better now.The main is a pain really did help me and taught me some nice new methods on how to work with java projects!Thanks a lot for that!
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I'm glad, It helped you and you made good changes, well done

    Only few things, IMO:
  • Can you tell why did you create separate class MyGame? If you don't have any code in that class you can happily move run method in MyGame class. No need of Logic class.

  • stelios papamichael wrote:As for the message before the input,i was planning on doing so afterwards!
    When your plan will be implemented?   because I ran your code and kept waiting something to happen on output screen then realised, it might be yet in your plan hahaha. Just jokes a part.

  • You can write this code in run method, I think no need to declare them in class.

  • Do you really need to pass String[] args to run method? I don't think so. Never used it. See in below code in MyGame

  • You can declare these String variables as private final and initialize in class i.e. the place where you declared Scanner sc  and String userChoice.
  • And remember name of final variables are written in capital letters, in your case ROCK, PAPER. Other example having two words like  EMP_SALARY etc
  •  
    Campbell Ritchie
    Marshal
    Posts: 55681
    161
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    stelios papamichael wrote:. . . My scanner variables are outside of my run method(new method inside a new class)so that there are no errors.However,whenever i try to put system.out.println(); outside of the run method,i . . .
    The declaration of a Scanner is a field declaration and (probably) initialisation. The print instruction is a statement and statements are not permitted outside methods or similar. Fields must be outside methods because they belong to the object not to the method. If you try declaring things inside methods, they turn into local variables.
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:I'm glad, It helped you and you made good changes, well done

    Only few things, IMO:
  • Can you tell why did you create separate class MyGame? If you don't have any code in that class you can happily move run method in MyGame class. No need of Logic class.

  • stelios papamichael wrote:As for the message before the input,i was planning on doing so afterwards!
    When your plan will be implemented?   because I ran your code and kept waiting something to happen on output screen then realised, it might be yet in your plan hahaha. Just jokes a part.

  • You can write this code in run method, I think no need to declare them in class.

  • Do you really need to pass String[] args to run method? I don't think so. Never used it. See in below code in MyGame

  • You can declare these String variables as private final and initialize in class i.e. the place where you declared Scanner sc  and String userChoice.
  • And remember name of final variables are written in capital letters, in your case ROCK, PAPER. Other example having two words like  EMP_SALARY etc


  • Thanks! Now : I created a new class not the MyGame one(this existed),the Logic one because in main is a pain they suggested that you put all your logic in another class and create another method containing all of it and then just create an instance of the Logic class and use the run method for example to run the application.This should be done because we do not want to have a bunch of code inside the main method(that's what they meant if i am correct).

    Also,i asked above too(i think) on how to print a message to the user outside of the run method because i cannot do that inside since the input from the user is taken before the run method(outside of it,inside the Logic class).When i move my Scanner and userInput variables inside the run method i get an error telling me that Scanner is leaking and it is never closed for some reason.I do not quite understand what it tries to tell me(there is a semicolon at the end of it :P).

    Furthermore,if i do not pass String[] args to the run method,i also get an error about it.
    As for the capital letters of final variables, i will fix this right away(did not know i was supposed to do it that way,so thanks a lot again ).
     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Campbell Ritchie wrote:
    stelios papamichael wrote:. . . My scanner variables are outside of my run method(new method inside a new class)so that there are no errors.However,whenever i try to put system.out.println(); outside of the run method,i . . .
    The declaration of a Scanner is a field declaration and (probably) initialisation. The print instruction is a statement and statements are not permitted outside methods or similar. Fields must be outside methods because they belong to the object not to the method. If you try declaring things inside methods, they turn into local variables.


    I had no idea, thanks for the info man it should help me a lot in the future!
    Then, how can i print a message to the console before the userInput variable? (if it is possible)
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ohh yes later you created Logic class MyGame was already there, yes you can have that two classes, I was thinking the alternative solution of main is a pain.
    When i move my Scanner and userInput variables inside the run method i get an error telling me that Scanner is leaking and it is never closed for some reason.

  • It may be warning not error. Never close a Scanner pointing to System.in, not even if Eclipse or any IDE complains about resource leaks. If you close that it closes input stream permanently so you can't take input from user in this running application. Read discussion about closing scanner here

  • Furthermore,if i do not pass String[] args to the run method,i also get an error about it.
  • Because doing this
  • will search for run method without parameter so you also have to remove that String[]arg parameter from run method.

  • Campbell answered you about print statement why it gives error if you write it outside of method or constructor or block.

  • Campbell Ritchie wrote:Fields must be outside methods because they belong to the object not to the method. If you try declaring things inside methods, they turn into local variables.
  • I think we better declare variables as close as possible to where they are used. In this case If the scanner is not used anywhere than run method then I think would be good to declare it in run method and yes that becomes local variable. Please Campbell correct me If I'm wrong, if wrong then why?. About String userChoice, It suppose to be in run method because used in run method only so better declare where it is used. Yes I do agree with String rock, paper and scissors these need to be declared outside of method because they belong to object.


  • Then, how can i print a message to the console before the userInput variable? (if it is possible)
  • If you keep userChoice declaration in class i.e. outside of run method then just declare String userChoice; like this in class and take input from user like this userChoice = sc.nextLine(); in run method before Math.random() line. Don't forget to write code to print message before taking input from user in run method.

  •  
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:Ohh yes later you created Logic class MyGame was already there, yes you can have that two classes, I was thinking the alternative solution of main is a pain.
    When i move my Scanner and userInput variables inside the run method i get an error telling me that Scanner is leaking and it is never closed for some reason.

  • It may be warning not error. Never close a Scanner pointing to System.in, not even if Eclipse or any IDE complains about resource leaks. If you close that it closes input stream permanently so you can't take input from user in this running application. Read discussion about closing scanner here

  • Furthermore,if i do not pass String[] args to the run method,i also get an error about it.
  • Because doing this
  • will search for run method without parameter so you also have to remove that String[]arg parameter from run method.

  • Campbell answered you about print statement why it gives error if you write it outside of method or constructor or block.

  • Campbell Ritchie wrote:Fields must be outside methods because they belong to the object not to the method. If you try declaring things inside methods, they turn into local variables.
  • I think we better declare variables as close as possible to where they are used. In this case If the scanner is not used anywhere than run method then I think would be good to declare it in run method and yes that becomes local variable. Please Campbell correct me If I'm wrong, if wrong then why?. About String userChoice, It suppose to be in run method because used in run method only so better declare where it is used. Yes I do agree with String rock, paper and scissors these need to be declared outside of method because they belong to object.


  • Then, how can i print a message to the console before the userInput variable? (if it is possible)
  • If you keep userChoice declaration in class i.e. outside of run method then just declare String userChoice; like this in class and take input from user like this userChoice = sc.nextLine(); in run method before Math.random() line. Don't forget to write code to print message before taking input from user in run method.



  • Alright,now everything has been fixed and improved so that the program works like a char.Thanks guy you are amazing,thank you for your patience and help.I've learned a lot of new things already which is amazing!Thanks again ;) :p !
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    You're welcome
     
    Carey Brown
    Bartender
    Posts: 2985
    46
    Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Excellent job of working through the issues.
    Now that you've got it working I feel it's ok to present another approach.
    This makes use of switch()'s ability to switch on Strings.
    This handles ties.
    And eliminates redundant use of println()'s.

     
    stelios papamichael
    Ranch Hand
    Posts: 93
    C++ Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Carey Brown wrote:Excellent job of working through the issues.
    Now that you've got it working I feel it's ok to present another approach.
    This makes use of switch()'s ability to switch on Strings.
    This handles ties.
    And eliminates redundant use of println()'s.



    WOW,this seems a lot simpler,i did not know i could use strings in switch statements like this.Thanks for the awesome tip m8!
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thank you Carey Brown, It was really awesome 
     
    Campbell Ritchie
    Marshal
    Posts: 55681
    161
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ganesh Patekar wrote:. . . Please Campbell correct me If I'm wrong, if wrong then why? . . .
    I wasn't speaking specifically about the Scanner field, but about fields in general.

    The Scanner field, of course, belongs in another class, which you use as a utility class, but that is probably beyond OP's experience to write.
     
    Ganesh Patekar
    Bartender
    Posts: 696
    23
    Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    @ Campbell Oh yes Agreed 
     
    It is sorta covered in the JavaRanch Style Guide.
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!