If statement with String comparison fails

Question

I really don't know why the if statement below is not executing:

if (s == "/quit")
{
    System.out.println("quitted");
}

Below is the whole class.

It is probably a really stupid logic problem but I have been pulling my hair out over here not being able to figure this out.

Thanks for looking :)

class TextParser extends Thread {
    public void run() {
        while (true) {
            for(int i = 0; i < connectionList.size(); i++) {
                try {               
                    System.out.println("reading " + i);
                    Connection c = connectionList.elementAt(i); 
                    Thread.sleep(200);

                    System.out.println("reading " + i);

                    String s = "";

                    if (c.in.ready() == true) {
                        s = c.in.readLine();
                        //System.out.println(i + "> "+ s);

                        if (s == "/quit") {
                            System.out.println("quitted");
                        }

                        if(! s.equals("")) {
                            for(int j = 0; j < connectionList.size(); j++) {
                                Connection c2 = connectionList.elementAt(j);
                                c2.out.println(s);
                            }
                        }
                    }
                } catch(Exception e){
                    System.out.println("reading error");
                }
            }
        }
    }
}

Solution 1

In your example you are comparing the string objects, not their content.

Your comparison should be :

if (s.equals("/quit"))

Or if s string nullity doesn't mind / or you really don't like NPEs:

if ("/quit".equals(s))

Solution 2

To compare Strings for equality, don't use ==. The == operator checks to see if two objects are exactly the same object:

In Java there are many string comparisons.

String s = "something", t = "maybe something else";
if (s == t)      // Legal, but usually WRONG.
if (s.equals(t)) // RIGHT
if (s > t)    // ILLEGAL
if (s.compareTo(t) > 0) // also CORRECT>

Solution 3

Strings in java are objects, so when comparing with ==, you are comparing references, rather than values. The correct way is to use equals().

However, there is a way. If you want to compare String objects using the == operator, you can make use of the way the JVM copes with strings. For example:

String a = "aaa";
String b = "aaa";
boolean b = a == b;

b would be true. Why?

Because the JVM has a table of String constants. So whenever you use string literals (quotes "), the virtual machine returns the same objects, and therefore == returns true.

You can use the same "table" even with non-literal strings by using the intern() method. It returns the object that corresponds to the current string value from that table (or puts it there, if it is not). So:

String a = new String("aa");
String b = new String("aa");
boolean check1 = a == b; // false
boolean check1 = a.intern() == b.intern(); // true

It follows that for any two strings s and t, s.intern() == t.intern() is true if and only if s.equals(t) is true.

Solution 4

If you code in C++ as well as Java, it is better to remember that in C++, the string class has the == operator overloaded. But not so in Java. you need to use equals() or equalsIgnoreCase() for that.

Solution 5

You shouldn't do string comparisons with ==. That operator will only check to see if it is the same instance, not the same value. Use the .equals method to check for the same value.

View more solutions

Comments

[palantus]

Too bad we can't give +1 for edits.

[Asaf]

how come it's tagged 'multithreading'?

[ReneS]

I prefer s.equals("/quit"). It is cosmetically but I want to have the important current data (s) to be visible easily.

[Anne Porosoff]

s.equales("/quit") will throw a NullPointerException is s is null, "/quit".equals(s) will never throw an NPE

[starblue]

So "/quit".equals(s) will mask bugs where s is unintentionally null.

[ReneS]

Well, another reason to have it s.equals()... might depend on the code, but the NPE is one reason to keep the current context upfront.

[Real Red.]

@ReneS whatever string/context s is I'd still do "<data>".equals(s) for I know <data> won't be NULL whereas s might. But if s.equals(s2) then you are right, but then again you'll have to surround it with if(s!= null){ block.

[Romain Hippeau]

Checks to see that the references are the same.