Java if Statement under enhanced for loop did not work

Question

This is a code fragment that I am working with for a logon system using hashset Users. I checked the enhanced for loop part it works but the if statement seems like it never works out, even if the values are equal.

if (comm.equals("Sign in")) {
    user urs = new user(p_name, p_pass, p_id );

    Iterator<user> iter = Users.iterator();
    for(user obj : Users) {
        if (p_u == obj.getUsername()&& p_pw == obj.getPassword ()&& p_sn== obj.getStudentID()) {
            JOptionPane.showConfirmDialog(
                jf, 
                "Success", "Success",
                JOptionPane.DEFAULT_OPTION);
            break;
        } else {
            log_in.setText("Try Again");
            exit.setText("Create User");

        }

if the username is a String, use String.equals(anotherstring) instead of == — Sara S, Apr 25 '14 at 14:35
@AnubianNoob OP's stating that the code inside `if` statement is never executed... — Luiggi Mendoza, Apr 25 '14 at 14:39
@SaraSeppola: I am pretty sure that your comment is the answer. — Bhesh Gurung, Apr 25 '14 at 14:39
The statement `Iterator iter = Users.iterator();` is not used (and not useful for "enhanced for loops") and `Users` appears to not follow common [Java naming conventions](http://en.wikipedia.org/wiki/Naming_convention_(programming)#Java) - only classes should start with upper case. To be honest, I'm quite confused -- the class appears to be called `user` and the collection `Users`.. this is pretty much the naming conventions backwards :D As for comparing strings in java, everything has been said by the other commenters already. — hiergiltdiestfu, Apr 25 '14 at 14:41
@BheshGurung no, we should vote to close for a duplicate Q/A... — Luiggi Mendoza, Apr 25 '14 at 14:42
@hiergiltdiestfu sometimes `Iterator` may be more useful than enhanced `for` loop (that uses `Iterator` for you behind the scenes). But in this case it won't do that much harm since JIT will remove it at execution time. — Luiggi Mendoza, Apr 25 '14 at 14:44
@Luiggi yeah sure, you need `Iterator` for on-the-fly-removal, but in that code example and in conjunction with an enhanced for loop it is neither used nor useful. The OP could be thinking that the statement is needed for the loop to work. I aimed to make sure this belief does not survive the comment section with him ;) — hiergiltdiestfu, Apr 25 '14 at 14:46
@hiergiltdiestfu first look at the problem: the `if` is not being accessed, solve it. After that, start improving the current code. Note that a decent IDE like Netbeans, Eclipse or IDEA will mark the `Iterator` line with a warning, and it's job from OP to listen to (or read) it or ignore it, we can as much say *pay attention to warnings, they're not to make your code yellow looking*. — Luiggi Mendoza, Apr 25 '14 at 14:49
@Luiggi way to be a motivator, really. The question had been answered already, I suggested additional points a reviewer would note, too. Given the poor style of the example, I was just trying to improve the OPs general understanding of Java and prevent misunderstandings. Never too late to save a C programmer. But yes, the extra statement did not do any harm, would perhaps even be optimized away at compile time (would it? the compiler can't know that it's side-effect free. Someone check please!), and yes, I did not (again) address the (already answered) central question -> You are right. :D — hiergiltdiestfu, Apr 25 '14 at 15:09
@hiergiltdiestfu as I've said, the code compiles and need a hand to work as expected. After you solve that, start improving it, that's all. And as I said in a previous comment, the JIT will remove the useless `Iterator` for you at execution time, so no need to make it a big deal :). — Luiggi Mendoza, Apr 25 '14 at 15:20

score 1 · Answer 1 · answered Apr 25 '14 at 14:40

Remember, in Java == operators test the equality of the value, in this case, two references. These two references do not point to the same memory location and therefore will never equate to true. Use the equals() method to test for equality of the values of strings.

score 1 · Answer 2 · answered Apr 25 '14 at 14:56

I think you need to changed it to something like this:

if (comm.equals("Sign in")) {
    user urs = new user(p_name, p_pass, p_id );

    Iterator<user> iter = Users.iterator();
    for(user obj : Users) {
        if (p_u.equalsIgnoreCase(obj.getUsername())&& p_pw.equals(obj.getPassword())&& p_sn.equals(obj.getStudentID())) {
            JOptionPane.showConfirmDialog(
                jf, 
                "Success", "Success",
                JOptionPane.DEFAULT_OPTION);
            break;
        } else {
            log_in.setText("Try Again");
            exit.setText("Create User");

        }

Rudi Kershaw · Accepted Answer · 2014-04-25T14:58:49.150

0

Assuming that p_u, p_pw and p_sn are String object references the code below is checking whether those two objects are the same object (or whether their refences to the object's position in memory are the same) rather than whether the String objects share the same character sequence.

if (p_u == obj.getUsername()&& p_pw == obj.getPassword ()&& p_sn== obj.getStudentID())

Instead, to check whether their character sequence matches up you should use the .equals() method. Example below;

if (p_u.equals(obj.getUsername())&& p_pw.equals(obj.getPassword ())&& p_sn.equals(obj.getStudentID()))

I hope this helps.

edited Apr 25 '14 at 14:58

answered Apr 25 '14 at 14:41

Rudi Kershaw

12,332
7
52
77

great job. this is exacly the solution I gave in my post 15min earlier but anyway – zpontikas Apr 25 '14 at 15:46
@zpontikas - Check the times on the answers. You answered almost 10 minutes after me. I remember thinking the same thing you just posted in the comments when you posted your answer. – Rudi Kershaw Apr 25 '14 at 15:46
Meh. You are right :P. I answered 2nd. its ok :D – zpontikas Apr 25 '14 at 15:50

Java if Statement under enhanced for loop did not work

3 Answers3