Deleting from Arraylist

Question

I wanted to optimize the code a bit. All trying to do is remove the product from the array. When I call the method deleteProduct(prod.getId()), it should delete product that I added first.

I can use the for loop, then how would delete the product in the array. Any pointers, please?

public void deleteProduct(int productId) throws ProductNotFoundException {

    Iterator<Product> it = allProducts.iterator();
    Product p= null;
    int pid = productId;
    int i = 0;

    if (!allProducts.isEmpty()) {
        while(it.hasNext()){
            p= it.next();
            i= allProducts.indexOf(p);
            if (p.getId().equals(productId)){
                i= allProducts.indexOf(p);
                allProducts.remove(i);
                System.out.println("Successfully removed the product " + pid);
                return;
            }
        }
    }   
        throw new ProductNotFoundException ("No Such Product");
}

Luiggi Mendoza · Accepted Answer · 2013-06-09T05:49:32.633

4

You can just use the iterator to remove the item by calling Iterator#remove:

while(it.hasNext()){
    p = it.next();
    if (p.getId().equals(productId)) {
         it.remove();
         System.out.println("Successfully removed the product " + pid);
         return;
    }
}
throw new ProductNotFoundException ("No Such Product");

From comment, using a for loop for iterator:

for(Iterator<Product> it = allProducts.iterator(); it.haNext(); ) {
    p = it.next();
    if (p.getId().equals(productId)) {
         it.remove();
         System.out.println("Successfully removed the product " + pid);
         return;
    }
}
throw new ProductNotFoundException ("No Such Product");

Probably you're asking how can I do this in a enhanced for loop? Answer is, you can't. But since enhanced for uses an iterator behind the scenes, the while loop approach would fit to your needs.

edited Jun 09 '13 at 05:49

answered Jun 09 '13 at 05:43

Luiggi Mendoza

85,076
16
154
332

Is not better using another list than arraylist for example linkedlist for removing? – nachokk Jun 09 '13 at 05:58
@nachokk the only difference would be time execution. In `ArrayList` implementation it will take *O(N)* and in `LinkedList` would be *O(1)* but that's not the main point in the question. – Luiggi Mendoza Jun 09 '13 at 06:00
Yeah i know but to add a note for a better implementation colega :-) – nachokk Jun 09 '13 at 06:04
@nachokk it will depend indeed. I would call this a micro premature optimization and remember that [premature optimization is the root of all evil](http://c2.com/cgi/wiki?PrematureOptimization) more info [here](http://www.codinghorror.com/blog/2009/01/the-sad-tragedy-of-micro-optimization-theater.html) and [here as well](http://1-800-magic.blogspot.com/2008/05/premature-optimization-is-root-of-all.html) colega hispanohablante. – Luiggi Mendoza Jun 09 '13 at 06:08

score 1 · Answer 2 · edited Jun 09 '13 at 05:55

1

You don't want to use allProducts.remove() because it will invalidate your iterator. You want to call it.remove(), which is guaranteed to leave the iterator valid. There is no need for any indexOf. You need to review what an iterator is doing: it is giving you access to the elements. You don't have to go back and fetch them with indexOf.

And, you don't need allProducts.isEmpty(), because it is redundant with the hasNext text in the while loop. If allProducts is indeed empty, the while condition will be false right at the beginning of the first iteration, and will be skipped.

edited Jun 09 '13 at 05:55

Luiggi Mendoza

85,076
16
154
332

answered Jun 09 '13 at 05:41

Andrew Lazarus

18,205
3
35
53

Thanks. My question is, how I can use for loop? – Newbie Jun 09 '13 at 05:43
You can convert the `while` to a `for` loop, but it is just as easy to read this way. Perhaps what you are asking is can you use a `for-each` loop (with the colon notation), and the answer is No, you can not remove in that type of loop. See [this](http://stackoverflow.com/questions/1196586/calling-remove-in-foreach-loop-in-java) very popular earlier SO question! – Andrew Lazarus Jun 09 '13 at 05:46
@LuiggiMendoza: Exactly where you remove it in your answer, with `it.remove()`. I was providing a delta rather than a complete answer, because I think this is a new Java programmer and he should rewrite his code for practice. – Andrew Lazarus Jun 09 '13 at 05:49

score 1 · Answer 3 · answered Jun 09 '13 at 05:45

1

I think you could change the List to a Map Implementation. If you use a Map, where product is mapped against a product id, you can directly remove the Map entry from Map just using product Id. Its more easy to retrieve also.

A map can perform far better than a loop. Only case is to take care for memory usage.

answered Jun 09 '13 at 05:45

Kris

8,680
4
39
67

+1. Probably correct for a production environment, but I think we're looking at someone learning Java here. – Andrew Lazarus Jun 09 '13 at 05:47

score 1 · Answer 4 · answered Jun 09 '13 at 06:14

1

Why you are using loop in the first place just override the .equal method in the product and let you method deleteProduct method take a product not id as parameter then just call allProduct.remove(Product p);

try this

package test;

public class Product {


    int id;

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + id;
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Product other = (Product) obj;
        if (id != other.id)
            return false;
        return true;
    }


}

and this

package test;

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

public class RemoveProduct {

    /**
     * @param args
     */
    java.util.List<Product> productList=new ArrayList<>();
    public static void main(String[] args) {
        // TODO Auto-generated method stub

    }


    public void deleteProduct(Product p)
    {

        productList.remove(p);
    }

}

answered Jun 09 '13 at 06:14

Mohammed Falha

967
8
22

and if you just have the id(you don't have the product objec) create an empty product and just assign the id you want . – Mohammed Falha Jun 09 '13 at 06:19
But that lead to define another methods that get the another field as parameter. if that the case we can make a generic delete method that take a map of than use reflection to get the value of that field and compare using it. – Mohammed Falha Jun 09 '13 at 07:21
That looks a bit overkill for the purpose of this exercise. Anyway, I would prefer to pass a `Comparator` (probably an anonymous implementation) that compares the product I'm seeking and then remove it with the iterator. For this solution, I would need to change only 2 lines in the code provided in my answer :) (not counting the `Comparator` implementation that must be at most 3 lines for this scenario). – Luiggi Mendoza Jun 09 '13 at 07:22
still you must define the Comparator for each field you want to compare to. – Mohammed Falha Jun 09 '13 at 07:49
If using an anonymous implementation it won't be as hard as you may think and it would be even better to read and maintain. If you still complain against this, you can have a bunch of `Comparator` instances in `Product` class definition. – Luiggi Mendoza Jun 09 '13 at 07:50

Deleting from Arraylist

4 Answers4