32

I need to delete some objects from an ArrayList if they meet a condition and I'm wondering which way could be more efficient.

Here's the situation: I have a class that contains an ArrayList containing some other objects. I have to iterate over this ArrayList and delete all elements meeting a certain condition. As far as I know, those would be my options to delete:

  1. Create a new ArrayList and add the elements that doesn't meet the condition. After the iteration, swap from the old arraylist to the new one without the elements.

  2. Create a new ArrayList and add the elements that meet the condition. After the iteration, use the removeAll() method passing the ArrayList with the objects to be deleted.

Is there a more efficient way to delete objects from an ArrayList?

Sam
  • 7,252
  • 16
  • 46
  • 65
Carlos Pastor
  • 979
  • 2
  • 11
  • 26
  • 2
    Unless you are really sure that performance is a problem at this particular point in your code, I would recommend to ignore efficiency. There are some other things you should consider, for example: Do you keep references to the original list elsewhere where the changes should be reflected? Then you couldn't use 1. And could you use `ArrayList.remove()`, i. e. does the semantic of `equals()` work as you need for the objects in the list? – Hanno Fietz Aug 21 '09 at 08:02
  • Well, the object that I'm talking about contains some arraylists and I'll have to do the same on all of them. I don't know if this could be a bottleneck (I haven't tested it), but I wanted to know how do you guys delete items to see if I there was better options. Answering your second question: yes, I can use the remove() method. – Carlos Pastor Aug 21 '09 at 08:09

13 Answers13

46

You could iterate backwards and remove as you go through the ArrayList. This has the advantage of subsequent elements not needing to shift and is easier to program than moving forwards.

RichardOD
  • 28,883
  • 9
  • 61
  • 81
  • Nice one, +1 :) Wonder if that performance improvement is actually noticeable. – Hanno Fietz Aug 21 '09 at 07:58
  • In some cases yes- probably one of those things to profile if you were concerned. Usually I take the strategy of building up another list of items to remove- it is easier for other programmers to understand. – RichardOD Aug 21 '09 at 07:59
  • Thanks, Richard, that's an interesting approach I've never thought of! +1 – Carlos Pastor Aug 21 '09 at 08:11
  • Yep, a great way to satisfy the requirements, and a common approach even in other languages. +1. – Decker Sep 22 '09 at 17:11
  • 1
    It doesn't matter if you find the element by traversing from the head or from the tail of the list- the actual deletion of that element will certainly involve shifting the rest of the members (which were to the right of the deleted element). Your solution does not make sense to me insofar as the actual deletion is concerned. Iterating backwards only benefits searching for the element if you know that this element resides somewhere towards the end of the list. Otherwise, your solution is no different from iterating forwards because the same amount of shifting takes place on deletion. – Dhruv Gairola Dec 25 '14 at 18:24
16

Another way: The Iterator has an optional remove()-method, that is implemented for ArrayList. You can use it while iterating.

I don't know though, which variant is the most performant, you should measure it.

starblue commented, that the complexity isn't good, and that's true (for removeAll() too), because ArrayList has to copy all elements, if in the middle is an element added or removed. For that cases should a LinkedList work better. But, as we all don't know your real use-cases the best is too measure all variants, to pick the best solution.

Mnementh
  • 50,487
  • 48
  • 148
  • 202
  • There is one caveat with `remove()` though: It may not remove the object you are currently looking at. Quoting from the JavaDoc: More formally, removes the element with the lowest index i such that (o==null ? get(i)==null : o.equals(get(i))) (if such an element exists)." So, depending on the objects in that list, it might not work as expected. Basically, if you could swap the ArrayList for a SortedSet without hurting your app, then you should be OK. – Hanno Fietz Aug 21 '09 at 07:55
  • 3
    This is what I see in javadoc for the remove() method of Iterator: "Removes from the underlying collection the last element returned by the iterator (optional operation). This method can be called only once per call to next. The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling this method." What am I missing? – Buhb Aug 21 '09 at 08:04
  • Thanks to all your answers, but I think Mnementh deserves the correct answer because he was the first who answered. – Carlos Pastor Aug 21 '09 at 08:36
  • 2
    This is worse than the two proposals in the question, because it leads to O(n²) runtime. – starblue Aug 21 '09 at 09:39
  • It also sucks because if an exception is thrown whilst iterating through the list then the list will be left in an inconsistent state. This could be an issue if the list is not local to the method. – pjp Aug 21 '09 at 10:09
  • pjp: That's also true of the `removeAll` approach, and most non-trivial mutations. starblue: O(n^2) is not necessarily slow. If the number of removals is relatively small it can fast. – Tom Hawtin - tackline Aug 21 '09 at 15:01
  • Yeah, ArrayList isn't the best solution, if you remove elements in the middle of it. But that is true for the removeAll()-approach too. Speaking about complexity, the new List should be working best. LinkedList also should work better on removes in the List. I add that to my answer. – Mnementh Aug 21 '09 at 18:38
12

Most performant would, I guess, be using the listIterator method and do a reverse iteration:

for (ListIterator<E> iter = list.listIterator(list.size()); iter.hasPrevious();){
    if (weWantToDelete(iter.previous()))  iter.remove();
}

Edit: Much later, one might also want to add the Java 8 way of removing elements from a list (or any collection!) using a lambda or method reference. An in-place filter for collections, if you like:

list.removeIf(e -> e.isBad() && e.shouldGoAway());

This is probably the best way to clean up a collection. Since it uses internal iteration, the collection implementation could take shortcuts to make it as fast as possible (for ArrayLists, it could minimize the amount of copying needed).

gustafc
  • 28,465
  • 7
  • 73
  • 99
5

Obviously, of the two methods you mention number 1 is more efficient, since it only needs to go through the list once, while with method number 2 the list has to be traversed two times (first to find the elements to remove, and them to remove them).

Actually, removing a list of elements from another list is likely an algorithm that's worse than O(n) so method 2 is even worse.

The iterator method:

List data = ...;

for (Iterator i = data.iterator(); i.hasNext(); ) {
    Object element = i.next();

    if (!(...)) {
        i.remove();
    }
}
Jesper
  • 202,709
  • 46
  • 318
  • 350
4

First, I'd make sure that this really is a performance bottleneck, otherwise I'd go with the solution that is cleanest and most expressive.

If it IS a performance bottleneck, just try the different strategies and see what's the quickest. My bet is on creating a new ArrayList and puting the desired objects in that one, discarding the old ArrayList.

Buhb
  • 7,088
  • 3
  • 24
  • 38
1

Unless you're positive that the issue you're facing is indeed a bottleneck, I would go for the readable

public ArrayList filterThings() {

    ArrayList pileOfThings;
    ArrayList filteredPileOfThings = new ArrayList();

    for (Thing thingy : pileOfThings) {
        if (thingy.property != 1) {
            filteredPileOfThings.add(thingy);
        }            
    }
    return filteredPileOfThings;
}
thomax
  • 9,213
  • 3
  • 49
  • 68
1

There is a hidden cost in removing elements from an ArrayList. Each time you delete an element, you need to move the elements to fill the "hole". On average, this will take N / 2 assignments for a list with N elements.

So removing M elements from an N element ArrayList is O(M * N) on average. An O(N) solution involves creating a new list. For example.

List data = ...;
List newData = new ArrayList(data.size()); 

for (Iterator i = data.iterator(); i.hasNext(); ) {
    Object element = i.next();

    if ((...)) {
        newData.add(element);
    }
}

If N is large, my guess is that this approach will be faster than the remove approach for values of M as small as 3 or 4.

But it is important to create newList large enough to hold all elements in list to avoid copying the backing array when it is expanded.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
1
int sizepuede= listaoptionVO.size();
for (int i = 0; i < sizepuede; i++) {
    if(listaoptionVO.get(i).getDescripcionRuc()==null){
        listaoptionVO.remove(listaoptionVO.get(i));
        i--;
        sizepuede--;
     }
}

edit: added indentation

Romain Francois
  • 17,432
  • 3
  • 51
  • 77
Raulitoxd
  • 11
  • 2
0

With an iterator you can handle always the element which comes to order, not a specified index. So, you should not be troubled with the above matter.

Iterator itr = list.iterator();
String strElement = "";
while(itr.hasNext()){

  strElement = (String)itr.next();
  if(strElement.equals("2"))
  {
    itr.remove();
  }
Rakshi
  • 6,796
  • 3
  • 25
  • 46
0

Whilst this is counter intuitive this is the way that i sped up this operation by a huge amount.

Exactly what i was doing:

ArrayList < HashMap < String , String >> results; // This has been filled with a whole bunch of results

ArrayList < HashMap < String , String > > discard = findResultsToDiscard(results);

results.removeall(discard);

However the remove all method was taking upwards of 6 seconds (NOT including the method to get the discard results) to remove approximately 800 results from an array of 2000 (ish).

I tried the iterator method suggested by gustafc and others on this post.

This did speed up the operation slightly (down to about 4 seconds) however this was still not good enough. So i tried something risky...

 ArrayList < HashMap < String, String>> results;

  List < Integer > noIndex = getTheDiscardedIndexs(results);

for (int j = noIndex.size()-1; j >= 0; j-- ){
    results.remove(noIndex.get(j).intValue());
}

whilst the getTheDiscardedIndexs save an array of index's rather then an array of HashMaps. This it turns out sped up removing objects much quicker ( about 0.1 of a second now) and will be more memory efficient as we dont need to create a large array of results to remove.

Hope this helps someone.

Aiden Fry
  • 1,672
  • 3
  • 21
  • 45
0

Maybe Iterator’s remove() method? The JDK’s default collection classes should all creator iterators that support this method.

Bombe
  • 81,643
  • 20
  • 123
  • 127
0

I have found an alternative faster solution:

  int j = 0;
  for (Iterator i = list.listIterator(); i.hasNext(); ) {
    j++;

    if (campo.getNome().equals(key)) {
       i.remove();
       i = list.listIterator(j);
    }
  }
-2

I'm good with Mnementh's recommentation.
Just one caveat though,

 ConcurrentModificationException

Mind that you don't have more than one thread running. This exception could appear if more than one thread executes, and the threads are not well synchronized.

Everyone
  • 2,366
  • 2
  • 26
  • 39
  • 1
    Not exactly - this will be thrown from a fail-fast collection if the underlying collection changes whilst iterating through it. For example you iterate over lst and one of your calls is to call remove on lst directly instead of using the remove method on ListIterator. – pjp Aug 21 '09 at 10:11