search in java ArrayList.

Question

I'm trying to figure out the best way to search a customer in an ArrayList by its Id number. The code below is not working; the compiler tells me that I am missing a return statement.

Customer findCustomerByid(int id){
    boolean exist=false;

    if(this.customers.isEmpty()) {
        return null;
    }

    for(int i=0;i<this.customers.size();i++) {
        if(this.customers.get(i).getId() == id) {
            exist=true;
            break;
        }

        if(exist) {
            return this.customers.get(id);
        } else {
            return this.customers.get(id);
        }
    }

}

//the customer class is something like that
public class Customer {
    //attributes
    int id;
    int tel;
    String fname;
    String lname;
    String resgistrationDate;
}

Answer 1

The compiler is complaining because you currently have the 'if(exist)' block inside of your for loop. It needs to be outside of it.

for(int i=0;i<this.customers.size();i++){
        if(this.customers.get(i).getId() == id){
            exist=true;
            break;
        }
}

if(exist)
    return this.customers.get(id);
else
    return this.customers.get(id);

That being said, there are better ways to perform this search. Personally, if I were using an ArrayList, my solution would look like the one that Jon Skeet has posted.

Answer 2

Customer findCustomerByid(int id){
    for (int i=0; i<this.customers.size(); i++) {
        Customer customer = this.customers.get(i);
        if (customer.getId() == id){
             return customer;
        }
    }
    return null; // no Customer found with this ID; maybe throw an exception
}

Answer 3

You're missing the return statement because if your list size is 0, the for loop will never execute, thus the if will never run, and thus you will never return.

Move the if statement out of the loop.

Answer 4

Others have pointed out the error in your existing code, but I'd like to take two steps further. Firstly, assuming you're using Java 1.5+, you can achieve greater readability using the enhanced for loop:

Customer findCustomerByid(int id){    
    for (Customer customer : customers) {
        if (customer.getId() == id) {
            return customer;
        }
    }
    return null; 
}

This has also removed the micro-optimisation of returning null before looping - I doubt that you'll get any benefit from it, and it's more code. Likewise I've removed the exists flag: returning as soon as you know the answer makes the code simpler.

Note that in your original code I think you had a bug. Having found that the customer at index i had the right ID, you then returned the customer at index id - I doubt that this is really what you intended.

Secondly, if you're going to do a lot of lookups by ID, have you considered putting your customers into a Map<Integer, Customer>?

Answer 5

Personally I rarely write loops myself now when I can get away with it... I use the Jakarta commons libs:

Customer findCustomerByid(int id){
    return (Customer) CollectionUtils.find(customers, new Predicate() {
     public boolean evaluate(Object arg0) {
      return ((Customer) arg0).getId()==id;
     }
    });
}

Yay! I saved one line!