Dealing with equality in an ArrayList in java

Question

Say I have this section of code:

for(int i = 0; i < accounts.size(); i++) {
    if(UserID.equals(accounts.get(i).getUserID())) {
        if(accounts.contains(accounts.get(i))) {
            if(UserPass.equals(accounts.get(i).getPassword())) {
                System.out.println("True");
            }
        } else {
            typePhrase("unrecognised userID: '" + UserID + "'");
        }
    } else {
        typePhrase("unrecognised userID: '" + UserID + "'");
    }
}

It goes through an arrayList filled with objects, that have an ID and a password. I get two inputs from the user, one is the userID, and the other is the password. What I want is for it to go through every possible object that is saved in that arrayList, and if it finds a match, print true into the console, the issue that I'm having is that if you type in something wrong, it prints an message that it is unrecognised for every object in the arrayList. It also prints the message for every object that there is in the arrayList -1 if you type one in right. What do you suggest I do?

User class:

public class User {
    String userID;
    String password;

    public User(String ID, String Pass) {
        userID = ID;
        password = Pass;
    }

    public String getUserID() {
        return userID;
    }

    public String getPassword() {
        return password;
    }
}

EDIT:

ArrayList<User> accounts = new ArrayList<User>();

Your User class doesn't implement equals or hashcode... Both are needed to use the contains method — OneCricketeer, Jul 28 '16 at 03:17
@cricket_007 why would i need to use the contain method on a user object? — Shaydoth, Jul 28 '16 at 03:19
@cricket_007 I think he is using `UserID` and `UserPass` as `String` variables, he hasn't called `equals` on `User` class — 11thdimension, Jul 28 '16 at 03:19
How would you ever expect `accounts.contains(accounts.get(i))` to be false? Check if the value at index `i` in the list is in the list. DUH! It's there!!! — Andreas, Jul 28 '16 at 03:20
No... `accounts.contains` compares `User` objects. You have not defined that behavior other than the default. — OneCricketeer, Jul 28 '16 at 03:22
@andreas oh sorry yes, you are right, but in my code, you don't have access to all of the user objects straight away, so that makes sure that you do, because the object would still exist even if you aren't able to use it straight away — Shaydoth, Jul 28 '16 at 03:25
Possible duplicate of [How does a ArrayList's contains() method evaluate objects?](http://stackoverflow.com/questions/2642589/how-does-a-arraylists-contains-method-evaluate-objects) — OneCricketeer, Jul 28 '16 at 03:26
@cricket_007 That other question has absolutely nothing to do with my question. I want to know how I can check each different object first, before printing an error message. — Shaydoth, Jul 28 '16 at 03:28
@Shaydoth By not printing anything *inside* the loop, perhaps? — Andreas, Jul 28 '16 at 03:29

score 5 · Answer 1 · answered Jul 28 '16 at 03:29

5

You should implement equals method in the User class:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;

    User user = (User) o;

    if (!getUserID().equals(user.getUserID())) return false;
    return getPassword().equals(user.getPassword());

}

Then you can create a new User with the typed information and just check if the list contains this User:

User user = new User("typedUserId", "typedPassword");
System.out.println(accounts.contains(user));

answered Jul 28 '16 at 03:29

Marcel Dias

4,271
1
14
10

What is `this` on the third line? – Shaydoth Jul 28 '16 at 03:30
1

`this` is a reference to the actual User object. – Marcel Dias Jul 28 '16 at 03:32
last 2 lines is better to be written as `return Objects.equals(getUserId(), user.getUserId()) && Objects.equals(getPassword(), user.getPassword())`. It is more readable (skipped bunch of if statements if you need to check for more than 2 fields, and is easier to read), and it is null-safe for attribute comparison. I'd even avoid calling `getUserId()` and access field directly, if it is possible. – Adrian Shum Jul 28 '16 at 03:42

score 1 · Accepted Answer · answered Jul 28 '16 at 03:37

This is a mistake I see newer programmers often make. You're searching through a list to see if any element meets some condition (e.g. ID and password match). If no element meets the condition, you do something that indicates an error.

But you can't tell if there's an error until you've gone through every element of the list. Therefore, any error message has to occur after the loop is completely done, right? Not in the middle of the loop. But you've put your "unrecognised" message in the middle of the loop. That can't work.

There are several common idioms to solve this, but here's a simple one:

boolean found = false;
for (whatever-your-loop-should-look-like) {
     if (the-current-element-meets-the-condition) {
         found = true;
         break;
     }
}
if (!found) {
     whatever-action-you-take-when-it-isn't-found;
}

OneCricketeer · Answer 3 · 2016-07-28T03:35:11.503

Remove this check... It makes no sense while you loop over those objects to check if the current object is inside the list of objects it came from.

if(accounts.contains(accounts.get(i))) {

Without this, your code prints True (but continues to check the rest of the list) when the userID and password match. Otherwise, the other message is printed. If you want to stop the loop when you print True, put break there.

To address the problem, though, User.equals() is not implemented, so the default way to compare objects is used (via the hashcode method).

You should implement that to compare equality of userID and password.

Adrian Shum · Answer 4 · 2016-07-28T03:39:01.287

0

Not sure if implementing equals() is a wise choice here. However, isn't that what you are trying to do as simple as:

boolean found = false;
for (User u : accounts) {
  if (userId.equals(u.getUserId()) && userPass.equals(u.getPassword()) {
    found = true;
    break;
  }
}

You may even use stream API if you are in Java 8+

accounts.stream().anyMatch(u -> userId.equals(u.getUserId()) 
                              && userPass.equals(u.getPassword());

edited Jul 28 '16 at 03:39

answered Jul 28 '16 at 03:34

Adrian Shum

38,812
10
83
131

"Not sure if implementing equals() is a wise choice here." Implementing `equals()` is always a wise choice. – Leo Izen Jul 28 '16 at 03:34
It's only wise if the `contains` method is needed, but for the sample code, it isn't – OneCricketeer Jul 28 '16 at 03:36
What I mean is implementing `equals()` for the sake of userId + password comparison. User, as an business entity, can contains other properties, that it is not wise to treat 2 users as `equals` simply because they have same userID and password – Adrian Shum Jul 28 '16 at 03:36

score 0 · Answer 5 · answered Jul 28 '16 at 03:50

Instead of printing true or not found, you can keep a boolean variable with whether you have found a match or not.

boolean found = false
for each value in the array
   if it's a match set found to true
   if it's not a match do nothing, i.e. continue to next position
if (found) print "true" else print "not found"

You can also break out of loop if you found a match, no need to keep checking for more matches.

boolean found = true
for each value in the array
   if it's a match set found to true and break out of loop
   if it's not a match do nothing, i.e. continue to next position
if (found) print "true" else print "not found"

Even better, you can move the code to a method that returns boolean and get rid of the variable.

boolean isThereAMatch() {
    for each value in the array
       if it's a match set return true
       if it's not a match do nothing, i.e. continue to next position
    return false
}

And you can call it to check what to print.

if (isThereAMatch()) print "true" else print "not found"

score 0 · Answer 6 · answered Jul 28 '16 at 04:03

You're struggling with whether to use contains() of List or to use a simple for loop. It can be done both ways, below is the code sample for both. For using the contains you'll have to add an equals() overridden method to User.

From the List.contains() documentation

Returns true if this list contains the specified element. More formally, returns true if and only if this list contains at least one element e such that (o==null ? e==null : o.equals(e)).

With For Loop

import java.util.*;

public class TestMain {
    public static void main (String[] args) {
        List<User> accounts = new ArrayList<User>();
        User user1 = new User("test", "test");
        User user2 = new User("test1", "test1");

        accounts.add(user1);
        accounts.add(user2);

        String userId = "test";
        String userPass = "test1";

        boolean matchFound = false;

        for(User account : accounts) {
            if(userId.equals(account.getUserID()) && userPass.equals(account.getPassword())) {
                System.out.println("True");
                matchFound = true;
            }
        }

        if(!matchFound) {
            System.err.println("unrecognised userID: '" + userId + "'");
        }
    }
}

class User {
    String userID;
    String password;

    public User(String ID, String Pass) {
        userID = ID;
        password = Pass;
    }

    public String getUserID() {
        return userID;
    }

    public String getPassword() {
        return password;
    }
}

With contains() and equals()

import java.util.*;

public class TestMain2 {
    public static void main (String[] args) {
        List<User> accounts = new ArrayList<User>();
        User user1 = new User("test", "test");
        User user2 = new User("test1", "test1");

        accounts.add(user1);
        accounts.add(user2);

        String userId = "test1";
        String userPass = "test1";

        boolean matchFound = accounts.contains(new User(userId, userPass));

        if(!matchFound) {
            System.err.println("unrecognised userID: '" + userId + "'");
        } else {
            System.out.println("True");
        }
    }
}

class User {
    String userID;
    String password;

    public User(String ID, String Pass) {
        userID = ID;
        password = Pass;
    }

    public String getUserID() {
        return userID;
    }

    public String getPassword() {
        return password;
    }

    @Override
    public boolean equals(Object user) {
        boolean isEqual = false;

        if(user != null && user instanceof User) {
            User userType = (User)user;
            boolean userIdMatched = (userID == null) ? userType.getUserID() == null : userID.equals(userType.getUserID());
            boolean passwordMatched = (password == null) ? userType.getPassword() == null : password.equals(userType.getPassword());

            isEqual =  userIdMatched && passwordMatched;
        }


        return isEqual;
    }
}

Dealing with equality in an ArrayList in java

6 Answers6