-2

I have two arraylists of users and I need to find a user with by searching them with user's email address. Here is my code. It works but can it be done more efficiently?

public String getUserId(ArrayList<User> newUsers, ArrayList<User> oldUsers, String email) {

    String userId = null;

    for (User user1 : newUsers) {
        if (user1.email.equals(email)) {
            userId = user1.uid;
        } 
    }

    if(userId == null){
        for (User user2 : oldUsers) {
            if (user2.email.equals(email)) {
                userId = user2.uid;
            }
        }
    }

    return userId;
}

And here is the User class.

public class User {

    public String uid;
    public String email;
    public String name;
    public String phoneNumber;

    public User() {
    }
}
Nicholas K
  • 15,148
  • 7
  • 31
  • 57
Daniel Foo
  • 73
  • 3
  • 9
  • Use a `Map` instead of `List`, with email as the key. If that is not an option, then `break` from the loop as soon a match is found. See also: parallel streams. – Andrew S Dec 18 '18 at 16:09
  • Depends whether your array lists are sorted or unsorted. If they are unsorted, its already the closest to best possible complexity. One thing you can do is, as soon as you find a match, stop iterating other elements and `return` immediately. – Sai Puli Dec 18 '18 at 16:11

6 Answers6

1

You need to terminate the loop by break as soon as you find the match:

  public String getUserId(ArrayList<User> newUsers, ArrayList<User> oldUsers, String email) {

    String userId = null;

    for (User user1 : newUsers) {
        if (user1.email.equals(email)) {
            userId = user1.uid;
            break;
        } 
    }

    if(userId == null){
        for (User user2 : oldUsers) {
            if (user2.email.equals(email)) {
                userId = user2.uid;
                break;
            }
        }
    }

    return userId;
}
forpas
  • 160,666
  • 10
  • 38
  • 76
0

Java 8 has Stream API which is similiar to Linq in C#

You can do something like this:

User foundUser = newUsers.stream()
  .filter((user) -> user.email == email)
  .findFirst()

if(foundUser == null){
foundUser = oldUsers.stream()
  .filter((user) -> user.email == email)
  .findFirst()
}

return foundUser.uid;

Basically its doing the same thing, but more short-hand a little neater/easier to read.

If you want to continue to use the for loops, add a break so you exit the for loop when a match is found

Jay Mason
  • 446
  • 3
  • 17
0

You could consolidate both lists into one and then just search that rather than iterating over both lists.

public String getUserId(ArrayList<User> newUsers, ArrayList<User> oldUsers, String email) {

    // create a new list
    List<User> consolidatedList = new ArrayList<>();

    // add both lists to it
    consolidatedList.addAll(newUsers);
    consolidatedList.addAll(oldUsers);

    // now just iterate over it
    User findAnyUser = consolidatedList.stream().filter(e -> e.getEmail().equals(email))
                                       .findFirst().orElse(null);

    return findAnyUser != null ? findAnyUser.getUid() : "Not Found";
}

Here once the first match is found the iteration of the list stops and the corresponding User object is returned. Now we check if the findAnyUser is null or not, depending on it we return the appropriate uid.

Nicholas K
  • 15,148
  • 7
  • 31
  • 57
0

Use Java 8 Stream API and Lambdas:

List<User> allUsers = new ArrayList<>();
    allUsers.addAll(oldUsers);
    allUsers.addAll(newUsers);

Optional<User> result = allUsers.stream()
                .filter(user -> email.equals(user.email))
                .findAny();
Daniel Man
  • 21
  • 5
0

One line solution (java 8+) :

return Stream
            .of(newUsers, oldUsers)
            .flatMap(Collection::stream)
            .filter(u -> Objects.equals(email, u.email))
            .findFirst()
            .orElse(new User()).email;

But it forces you to instantiate a user when no user is found.

Loïc
  • 113
  • 1
  • 1
  • 8
0

Find an Object in two Array Lists

This is the most cleanest way:

public int findRapperIndexInPairings(User user){
    int index1 = getPairing1().indexOf(user);
    int index2 = getPairing2().indexOf(user);
    int userIndex = -1;

    if(index1 > 1) userIndex = index1;
    if(index2 > 1) userIndex = index2;

    return userIndex;
}
Peter Palmer
  • 726
  • 11
  • 18