I have created a test class below. My main objective is to add an object to a given list only if the object's certain parameters (in this case name) do not match.

import java.util.ArrayList;
import java.util.List;

public class SmartAuto {
static Car car1 = new Car(1, "Lexus");
static Car car2 = new Car(2, "Tesla");
static Car car3 = new Car(3, "Lexus");

public static void main(String[] args) {
    List<Car> cars = new ArrayList<Car>();
    addNewCar(cars, car1);
    addNewCar(cars, car2);
    addNewCar(cars, car3);
    
    System.out.println(cars.size());
}

private static List<Car> addNewCar(List<Car> cars, Car car) {
    if (cars.size() == 0) {
        cars.add(car);
        return cars;
    }
    cars.forEach(c -> {
        if(c.name == car.name) {
            System.out.println("Can't add as car names are duplicate");
            return;
        } else {
            cars.add(car);
            return;
        }
    });
    
    return cars;
}

public static class Car {
    int id;
    String name;
    
    Car (int id,String name) {
        this.id = id;
        this.name = name;
    }
}

My problem is this is failing with the below exception that I don't understand :

Exception in thread "main" java.util.ConcurrentModificationException
at java.base/java.util.ArrayList.forEach(ArrayList.java:1542)
at Sample/main.SmartAuto.addNewCar(SmartAuto.java:25)
at Sample/main.SmartAuto.main(SmartAuto.java:14)

Also, I need to know if this code can be written in a better way in java 8?

CodePudding user response：

1. Check for existence before adding; do not add inside the forEach loop (this is causing the exception).
2. Use .equals to compare String objects, not ==.
3. You can use Stream#noneMatch for checking the condition. (For better performance, consider storing a Set of names that are already in the List.)

if (cars.stream().noneMatch(c -> c.name.equals(car.name))) cars.add(car);
else System.out.println("Can't add as car names are duplicate");
return cars;

CodePudding user response：

You are not supposed to modify the source list within the Stream's forEach.

The behavior of this method is unspecified if the action performs side-effects that modify the underlying source of elements, unless an overriding class has specified a concurrent modification policy.

Change your addNewCar method to identify if a duplicate exists first by looping through to the end of the list or exiting out when one exists.. and then add to the list. BTW.. a cleaner way you can accomplish this is by changing the collection to a Set and defining the Car object hashcode to be the same as name field hashcode and adding to a HashSet which then automatically stores unique cars.
But, limiting the answer to a quick fix in your code snippet, the following change to addNewCar should address the problem.

private static List<Car> addNewCar(List<Car> cars, Car car) {
    if (cars.size() == 0) {
        cars.add(car);
        return cars;
    }

    boolean duplicate = false;
    for (Car c : cars) {
        if (c.name == car.name) {
            System.out.println("Can't add as car names are duplicate");
            duplicate = true;
            break;
        }
    }
    if (!duplicate) {
        cars.add(car);
    }

    return cars;
}