Java - remove() function for ArrayList (I need help)

Question

how can i remove an element that i had added before in the ArrayList<> I create it like that:

public static ArrayList<Product> P = new ArraList<Product>();

the method I am using:

public void removeProduct(Product p) {
    P.remove(p); // this way, did not solve the problem 
}

// I did the (added the method) and it works and everything are fine, I hope someone could help to get the answer and thanks :)

public void deleteProduct(String ID) {
    System.out.println("Enter product id to delete: ");
    ID = input.next();
    for(Product m : s.P) {
        if(ID.equals(m.getID())) {
            s.P.remove(ID);
        }
        else {
            System.out.println("ID is not exist");
        }
    }
}

// and

public void removeProductToCart(Product p) {
    viewShoppingCart();
    System.out.println("Enter product id to remove it: ");
    String ID = input.next();
    for(Product r : s.P) {
        if(ID.equals(r.getID())) {
            s.P.remove(p);
        }
        else {
            System.out.println("ID is not exist");
        }
    }
}

What is the concrete problem. "I need help" doesn't tell us anything about the problem you're facing. What are you doing, what do you expect to happen, what happens instead? Be precise. — JB Nizet, Mar 24 '19 at 18:39
You write `I did the (added the method) and it works and everything is fine`. What are you need if is `all fine`? — Valentyn Hruzytskyi, Mar 24 '19 at 18:48
maybe an Exception is being thrown. If so, it would help to know about it (message, where, ...) — user85421, Mar 24 '19 at 18:59

Oleksii Zghurskyi · Answer 1 · 2019-03-24T18:54:58.093

You need to use iterator, otherwise you will get java.util.ConcurrentModificationException. The exception is thrown, because you are doing 2 operations on the list: iteration and removal.

So, you need something like this:

for (Iterator<Book> it = s.P.listIterator(); it.hasNext(); ) {
    Product r = it.next();
    if(ID.equals(r.getID())) {
        it.remove(r);
    }
}

Because, the root cause is performing 2 operations, there is another approach - just create a copy of the list on each step of iteration:

for(Product m : new ArrayList<>(s.P)) {
    if(ID.equals(m.getID())) {
        s.P.remove(m);
    }
}

Note: Because of performance considerations (quadratic memory usage and linear removal on each step), I don't recommend the last approach. I give this example only to stress out the underlying reason why java.util.ConcurrentModificationException is thrown.

score 1 · Accepted Answer · answered Mar 24 '19 at 18:49

2 problems:

s.P is a list of products, not strings, so calling remove(String) doesn't work.
Removing elements in a for-each loop will throw a ConcurrentModificationException

Possible solution:

public void removeProductToCart(Product p) {
    viewShoppingCart();
    System.out.println("Enter product id to remove it: ");
    String ID = input.next();
    Product toRemove = null;
    for(Product r : s.P) {
        if(ID.equals(r.getID())) {
            toRemove = r;
            break;
        }
    }
    if(toRemove == null) {
        System.out.println("ID is not exist");
    }
    else {
        s.P.remove(toRemove);
    }
}

This can be simplified if the passed argument is the product that needs to be removed.

The same logic can be applied to the first function:

public void deleteProduct(String ID) {
    System.out.println("Enter product id to delete: ");
    ID = input.next();
    Product toRemove = null;
    for(Product r : s.P) {
        if(ID.equals(r.getID())) {
            toRemove = r;
            break;
        }
    }
    if(toRemove == null) {
        System.out.println("ID is not exist");
    }
    else {
        s.P.remove(toRemove);
    }
}

Note: The method arguments currently serve no purpose. Why not use them instead of looping to find the product?

thanks you so much, you saved me, but there is another thing the shopping cart never deleted there, but thanks i can understand your way — Ahmedhussain, Mar 24 '19 at 19:29

Java - remove() function for ArrayList (I need help)

2 Answers2