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;
}
Personally I rarely write loops myself now when I can get away with it... I use the Jakarta commons libs:
Customer findCustomerByid(final int id){
return (Customer) CollectionUtils.find(customers, new Predicate() {
public boolean evaluate(Object arg0) {
return ((Customer) arg0).getId()==id;
}
});
}
Yay! I saved one line!
I did something close to that, the compiler is seeing that your return statement is in an If() statement. If you wish to resolve this error, simply create a new local variable called customerId before the If statement, then assign a value inside of the if statement. After the if statement, call your return statement, and return cstomerId. Like this:
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;
}
int customerId;
if(exist) {
customerId = this.customers.get(id);
} else {
customerId = this.customers.get(id);
}
}
return customerId;
}
Even if that topic is quite old, I'd like to add something.
If you overwrite equals
for you classes, so it compares your getId
, you can use:
customer = new Customer(id);
customers.get(customers.indexOf(customer));
Of course, you'd have to check for an IndexOutOfBounds
-Exception, which oculd be translated into a null pointer or a custom CustomerNotFoundException
.
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.
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>
?
In Java 8:
Customer findCustomerByid(int id) {
return this.customers.stream()
.filter(customer -> customer.getId().equals(id))
.findFirst().get();
}
It might also be better to change the return type to Optional<Customer>
.
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
}
Source: Stackoverflow.com