Java Stream code improvement

Question

I have a map of cars Map<String, List<Car>> which can be returned by getCarsMap() method where key is model of car , and value is list of cars of same model.

I would like to improve Car addCar(Car car) function. For now I wrote this a bit ugly peace of code

public Car addCar(Car car) {
    if(getCarsMap().entrySet().stream().anyMatch(es -> es.getKey().equals(car.getModel()))){
        if(!getCarsMap().get(car.getModel()).contains(car)){
            getCarsMap().get(car.getModel()).add(car);
        }
    }else{
        List<Car> tmpList = new ArrayList<Car>();
        tmpList.add(car);
        getCarsMap().put(car.getModel(), tmpList) ;
    }
    return car;
}

Also, need to write something similar for removeCar(Car car) but hope it's gonna be easier with some nice addCar method as a template.

Answer 1

The tips:

• Use Set instead of List because Set doesn't allow duplicates.

• Use contains to check if an element is in a collection, regardless of whether it's a List or a Set. (Car class must have a equals method that compares the field values.)

• Return a boolean to indicate if the collection was modified as a result of the operation. This is what Java collections do.

The code:

public boolean addCar(Car car) {
    Map<String, Set<Car>> carsMap = getCarsMap();
    if (!carsMap.containsKey(car.getModel())) {
        carsMap.put(car.getModel(), new HashSet<>(Arrays.asList(car)));
        return true;
    }
    if (!carsMap.get(car.getModel()).contains(car)) {
        return carsMap.get(car.getModel()).add(car);
    }
    return false;
}

Answer 2

My idea - rely on compute method of Map.

So my method populateMapWith process current map according to your case:

• if value list is empty - create new one, add element, add to map

• if present, just add element to current list

as well as removeFromMap according to removing.

I illustrated it with Object class, just to simplify the case

public class BaseTest {

    private Map<String, List<Object>> myMap = new HashMap<>();

    @Test
    public void testName() throws Exception {
        Object car1 = new Object() {
            @Override
            public String toString() {
                return "car1";
            }
        };

        Object car2 = new Object() {
            @Override
            public String toString() {
                return "car1";
            }
        };

        populateMap(car1);
        populateMap(car2);

        assertEquals(2, myMap.get("car1").size());

        removeFromMap(car2);

        assertEquals(1, myMap.get("car1").size());
    }

    private void populateMap(Object o) {
        myMap.computeIfAbsent(o.toString(), key -> new ArrayList<>()).add(o);
    }

    private void removeFromMap(Object o) {
        myMap.computeIfPresent(o.toString(), (key,list) -> { list.remove(o); return list;});
    }
}

UPD : improved methods after discussion with Holger

Answer 3

if(!getCarsMap().get(car.getModel()).contains(car))

It seems that you do not wish to have duplicates. So instead of a List, you must use a Set, that does not add duplicate elements, like:

Map<String, Set<Car>> carmap = getCarsMap();

getBoatsMap().put(car.getModel(), tmpList) ;

This doesn't make sense. Putting a car in a map that is supposed to have boats is bewildering. However, I assume to go with it.

So we can have:

Set<Cars> tempcar = new HashSet<>();
tempcar.add(car);
getBoatsMap().put(car.getModel(),tempcar);

public Car addCar(Car car)

The return type of addCar method. A method returns the object that it manipulates. Here, the map is manipulated and not the object. It should be something like:

public Map<String,Set<Car>> addCar(Car car)

So we can have the following code:

 public HashMap<String,Set<Car>> addCar(Car car){
        Map<String, Set<Car>> carmap = getCarsMap();
        if(carmap.containsKey(car.getModel())){
            HashSet<Car> carset = carmap.get(car.getModel());
            carset.add(car);
            map.put(car.getModel(),carset);
        }else{
            carmap = getBoatsMap();
            Set<Cars> tempcar = new HashSet<>();
            tempcar.add(car);
            carmap.put(car.getModel(),tempcar);
        }
        return carmap;
   }

This can be the basic template for you to move on. However, this also can be further optimized. A deeper study of Java Collections API and Java 8 can provide you with better ideas.

For, removeCar(), the method signature can be:

public Car removeCar(Car) 

UPDATE I see in comment that getBoatsMap() is a mistake. So the code is:

public Map<String, Set<Car>> addCar(Car car){
    Map<String, Set<Car>> carmap = getCarsMap();
    if(carmap.containsKey(car.getModel())){
        HashSet<Car> carset = carmap.get(car.getModel());
        carset.add(car);
        map.put(car.getModel(),carset);
    }else{
        Set<Cars> tempcar = new HashSet<>();
        tempcar.add(car);
        carmap.put(car.getModel(),tempcar);
    }
    return carmap;
}