1

Java newbie and I'm stuck on something that seems really simple. I wrote an algorithm for a game I'm building and, using TDD, all of my tests pass, but the actual game is not returning the right feedback. Instead, whenever I guess a correct letter, it returns a "w" and should return a "b".

Background The game starts by generating a 4 character random code. The objective of the game is to enter a guess and try to match the secret code. The feedback class is supposed to tell me if my guess is hot or cold compared to the code.

I pass the guess and secret code to the class and calculate the feedback. The guess is a string that I use "split" to turn into a String array. The secret code is also a string Array that I randomly generate 4 different string values for. I tried checking to see if one element from both the guess and secret code are strings and got "true" on both.

Problem Problem is that my exact matches if statement never gets executed. I read that you should use equals() instead and also tried that but get an error.

Can anyone help?

Here is my Feedback class: public class Feedback {

      public String[] guess;
      public String[] secretCode;
      public String[] feedback;

      public String[] get(String[] guess, String[] secretCode) {
        this.guess = Arrays.copyOf(guess, guess.length);
        this.secretCode = Arrays.copyOf(secretCode, secretCode.length);
        feedback = new String[this.secretCode.length];
        findExactMatches();
        findNearMatches();
        findNoMatches();
        sortFeedback();
        return feedback;
      }

      public void findExactMatches() {
        for (int i = 0; i < guess.length; i++) {
          if (guess[i] == secretCode[i]) {
            feedback[i] = "b";
            guess[i] = "x";
            secretCode[i] = "x";
          }
        }
      }

      public void findNearMatches() {
        for (int i = 0; i < guess.length; i++) {
          if ( Arrays.asList(secretCode).contains(guess[i]) && guess[i] != "x" ) {
            feedback[i] = "w";
            int matched_symbol_index = Arrays.asList(secretCode).indexOf(guess[i]);
            secretCode[matched_symbol_index] = "x";
          }
        }
      }

      public void findNoMatches() {
        for (int i = 0; i < guess.length; i++) {
          if ( feedback[i] == null ) {
            feedback[i] = " ";
          }
        }
      }

      public void sortFeedback() {
        Arrays.sort(feedback);
      }
    }
Kelly
  • 619
  • 8
  • 18
  • *"I read that you should use equals() instead and also tried that but get an error."*. It would help if you told us what that error was, because FIXING it is probably the only real solution. – Stephen C May 26 '13 at 04:03
  • -1; question's been asked and answered half a billion times already here and elsewhere on Google. – Nico May 26 '13 at 04:44
  • 1
    As a side note, you should avoid the repeated call to Arrays.asList() inside the for loops. Either go with raw arrays and write you own contains() and indexOf() methods; or stick with some ArrayList. – Gyro Gearless May 26 '13 at 08:57
  • Thank you Gyro! That was a helpful tip. I took your advice and rolled my own method. – Kelly May 26 '13 at 14:07

3 Answers3

3

When comparing Strings, use equals instead of ==. String.equals will compare the content, while == will compare the identity (that is if they are the same object or not).

So,

guess[i] == secretCode[i]

should be

guess[i].equals(secretCode[i])

If you want to learn more, then you could do == if you interned the string. But that's completely bonus content.

Edit: And also, as johnchen902 said in the comment:

And guess[i] != "x" should be !guess[i].equals("x"). (in findNearMatches())

Community
  • 1
  • 1
zw324
  • 26,764
  • 16
  • 85
  • 118
  • 1
    And `guess[i] != "x"` should be `!guess[i].equals("x")`. (in `findNearMatches()`) – johnchen902 May 26 '13 at 04:00
  • Thanks so much! I had read about "equals" several times, but I guess I read it wrong. I was trying to do equals(guess[i], secretCode[i]) instead of the way you described. – Kelly May 26 '13 at 14:04
3

These are strings so you should use equals, i.e. guess[i].equals(secretCode[i])

guess[i] == secretCode[i] will compare reference of the two string ("operator compares the objects’ location(s) in memory") which you do not want.

Please look at this for more -- http://www.programmerinterview.com/index.php/java-questions/java-whats-the-difference-between-equals-and/

And same applies for guess[i] != "x" so you will use ! guess[i].equals("x")

Bill
  • 5,263
  • 6
  • 35
  • 50
3

Replace the guess[i] == secretCode[i] with guess[i].equals(secretCode[i]) and the guess[i] != "x" with !guess[i].equals("x") .

== tests for reference equality.

.equals() tests for value equality.

You can find more info here

public String[] guess;
      public String[] secretCode;
      public String[] feedback;

      public String[] get(String[] guess, String[] secretCode) {
        this.guess = Arrays.copyOf(guess, guess.length);
        this.secretCode = Arrays.copyOf(secretCode, secretCode.length);
        feedback = new String[this.secretCode.length];
        findExactMatches();
        findNearMatches();
        findNoMatches();
        sortFeedback();
        return feedback;
      }

      public void findExactMatches() {
        for (int i = 0; i < guess.length; i++) {
          if (guess[i].equals(secretCode[i]) ) {
            feedback[i] = "b";
            guess[i] = "x";
            secretCode[i] = "x";
          }
        }
      }

      public void findNearMatches() {
        for (int i = 0; i < guess.length; i++) {
          if ( Arrays.asList(secretCode).contains(guess[i]) && !guess[i].equals("x") ) {
            feedback[i] = "w";
            int matched_symbol_index = Arrays.asList(secretCode).indexOf(guess[i]);
            secretCode[matched_symbol_index] = "x";
          }
        }
      }
Community
  • 1
  • 1
Petros Tsialiamanis
  • 2,718
  • 2
  • 23
  • 35
  • Thank you so much! I see what I was doing wrong now. And thanks so much for the description of how the two methods compare and the link. – Kelly May 26 '13 at 14:04