Filtering out the correct car from a list of cars and retrieving only one car

Question

I am filtering out a List<Car> based on the brand. Since each car is made by one brand, I want to make sure my method getCar() is called and just a single car is returned. If it returns more than one car, throw an exception.

This is a simplified example, my current implementation is as follows:

I have a list of cars stored in an array list, please assume this list cars already has car object stored in them. Also, in my Car class implementation, I have a method call isCorrectBrand() which check if the brand matches.

List<Car> cars;

public Car getCar(String brand) {
    List<Car> filteredCars = new ArrayList<>();
    cars.stream()
            .filter(car -> car.isCorrectBrand(brand))   
            .forEach(filteredCars::add);

    if (filteredCars.size() > 1) {
       throw new Exception("");
    } else {
      return filteredCars.get(0);
    }
}

My approach is to first loop through the cars list, store them in a temp list, and if the list is size one, that means everything is fine, otherwise it indicates something wrong with either the brand message or the isCorrectBrand() implementation.

My questions are:

1. Is the exception thrown an appropriate one, if not what exception is appropriate for this use?

2. Is there a better way of writing this?

Answer 1

Your solution is inefficient, for two reasons:

• If there are multiple matches, the filteredCars lists gets unnecessarily populated.
• It iterates over the entire list, even though it could stop immediately when a second match is found.

The appropriate exception type depends on your application and its purpose, for example:

• If the caller is wrong to request a brand of which multiple cars exist, then you could use IllegalArgumentException.
• If the cars list is wrong to contain multiple cars of the requested brand, then you could use IllegalStateException.
• If you want to give the caller a chance to recover from the bad call, then you should use a checked custom exception type, catch it and handle it.

Btw, it seems you forgot to handle the case of no matching cars, which would result in an exception when you call .get(0).

As a minor remark, the name isCorrectBrand seems tedious. Simply isBrand would be more natural.

Consider this alternative:

public Car getCar(String brand) {
    return cars.stream()
            .filter(car -> car.isBrand(brand))
            .reduce((a, b) -> {
                throw new IllegalArgumentException("more than one matching cars");
            })
            .orElseThrow(() -> new IllegalArgumentException("no matching cars"));
}