Enhanced for loop to remove objects from ArrayList

Question

I have two ArrayLists from a Parse Query that I am comparing with an enhanced for loop. I want to check to see if there is a matching ObjectId in the 2 Lists, if so, I want to remove that Object from "allDropList". All other components are working properly, I believe the problem to be with my enhanced for loop syntax. Here is my code, thanks! Note: I have experimented with "if" and "while", with no success.

public ArrayList<DropItem> filterDrops(ArrayList <DropItem> hasRelationList , ArrayList <DropItem> allDropsList){

    for(DropItem dropItemAll : allDropsList) {

        for(DropItem dropItemRelation  : hasRelationList) {

           /*if*/ while(dropItemAll.getObjectId().equals(dropItemRelation.getObjectId())) {
                  allDropsList.remove(dropItemAll);
            }
        }
    }
    return allDropsList;
}

moreover, if I understand well, `while` is a nonsense here. `if` should be used — Rémi F, Nov 17 '15 at 22:35
Yes, I tried both and realized neither one was working. I actually thought that changing it to while would have fixed it, but I was mistaken. — Kevin Hodges, Nov 17 '15 at 22:37
You shouldn't really remove items from a list using a for loop -- use an iterator instead. http://stackoverflow.com/questions/1196586/calling-remove-in-foreach-loop-in-java — BlakeP, Nov 17 '15 at 22:38
@BlakeP You may be correct, I was told by my mentor to use an enhanced for loop as I will have many Drop Objects eventually. — Kevin Hodges, Nov 17 '15 at 22:41
I would have created different ArrayList for result and add items in it to return later instead of removing items — AAnkit, Nov 17 '15 at 22:52

score 0 · Answer 1 · answered Nov 17 '15 at 22:41

0

The problem is you can't iterate over a list and remove something at the same time with a for loop. Have a look here for further explanation.

answered Nov 17 '15 at 22:41

Frank Andres

146
5

I see, Frank. In that case what do you recommend? – Kevin Hodges Nov 17 '15 at 22:43
@Kevin Hodges have a look at http://stackoverflow.com/questions/223918/iterating-through-a-list-avoiding-concurrentmodificationexception-when-removing – Frank Andres Nov 17 '15 at 22:46
Thanks for the link Frank, I will check this out. – Kevin Hodges Nov 17 '15 at 22:52

score 0 · Answer 2 · answered Nov 17 '15 at 22:45

0

Removing objects in list is safer is done in a reverse order:

  public ArrayList<DropItem> filterDrops(ArrayList <DropItem> hasRelationList ,ArrayList <DropItem> allDropsList){
    for(int i = allDropsList.size()-1; i >=0; i--) {
      for(int j = hasRelationList.size()-1; j >=0; j--) {
        if(allDropsList.get(i).getObjectId().equals(hasRelationList.get(j).getObjectId())) {
          allDropsList.remove(i);
        }
      }
    }
    return allDropsList;
  }

answered Nov 17 '15 at 22:45

Rémi F

1,327
12
25

Thanks for your input buz. I implemented this code, unfortunately the Drops in question are still not being removed from my allDropsList. – Kevin Hodges Nov 17 '15 at 22:52
Ok. Did you check by debugging that the inner test is behaving correctly? – Rémi F Nov 17 '15 at 23:46

Touf · Answer 3 · 2015-11-18T01:17:50.973

0

Or you can just simply do this:

    public ArrayList<DropItem> filterDrops(ArrayList <DropItem> hasRelationList , ArrayList <DropItem> allDropsList){

    ArrayList<DropItem> temp = new ArrayList<DropItem>(); //place the objects you want inside this arraylist
    for(DropItem dropItemAll : allDropsList) {
        int counter = 0;
        for(DropItem dropItemRelation  : hasRelationList) {

            if(dropItemAll.getId().equals(dropItemRelation.getId())) {
                break;
            }
            if(counter == hasRelationList.size()-1){
               temp.add(dropItemAll);
            }
            counter++;
        }
    }
    return temp;
}

edited Nov 18 '15 at 01:17

answered Nov 17 '15 at 22:58

Touf

106
1
6

This returned an empty list for me :( – Kevin Hodges Nov 17 '15 at 23:25
If it returned an empty list this means all objects inside both lists have same ids .. or at least one of the lists is empty. Try debugging to find out. – Touf Nov 17 '15 at 23:48
I had to logout of my app and back in to get any data to show, the list populates, but no Drops are being removed from my allDropsList, they are all there. – Kevin Hodges Nov 17 '15 at 23:54
Yes of course, I had a logical error. I edited the code now this should work. But it's better to use GreyBeardedGeek's answere. – Touf Nov 18 '15 at 01:23

score 0 · Answer 4 · answered Nov 17 '15 at 23:14

0

If you use an Iterator instead for the outer loop, you can remove items from the list while iterating, without resorting to other 'tricks'.

public ArrayList<DropItem> filterDrops(ArrayList <DropItem> hasRelationList, ArrayList <DropItem> allDropsList){
    Iterator<DropItem> allDropsIterator = allDropsList.iterator();

    while(allDropsIterator.hasNext()) {
        DropItem dropItemAll = allDropsIterator.next();

        for(DropItem dropItemRelation  : hasRelationList) {
          if(dropItemAll.getObjectId().equals(dropItemRelation.getObjectId()){
              allDropsIterator.remove(dropItemAll);
        }
    }

    return allDropsList;
}

answered Nov 17 '15 at 23:14

GreyBeardedGeek

29,460
2
47
67

Thanks for the input, I received: "remove() in iterator cannot be applied to " – Kevin Hodges Nov 17 '15 at 23:24
You should be calling remove() on the iterator (an Iterator), not on a DropItem. If you are doing that, then there must be another issue, as Iterator definitely supports the remove(T) method. – GreyBeardedGeek Nov 17 '15 at 23:28
dropItemAll is a , this won't allow me to use your code, as the "remove(dropItemAll); is illegal. Is there a way around this? – Kevin Hodges Nov 17 '15 at 23:35
dropItemAll is not a , since syntactically, there is no such thing. dropItem is a DropItem. And allDropsIterator is an Iterator, which most certainly does have a remove(DropItem) method. If you are still having problems, please post your current code, along with the full text of the compiler error. – GreyBeardedGeek Nov 17 '15 at 23:43
I posted my code along with the error. I am very new to this so I may have posted it in the wrong place :/ Let me know if you need to see anything else. – Kevin Hodges Nov 17 '15 at 23:59
Yes, sorry, it's iterator.remove() - it takes no argument, just removes the current item that the iterator is pointing to. I'll post a full working example: – GreyBeardedGeek Nov 18 '15 at 00:00

score 0 · Accepted Answer · answered Nov 18 '15 at 00:07

Here's the corrected answer using an Iterator. This is a full example that compiles and runs:

import java.util.ArrayList;
import java.util.Iterator;

 public class DropTest {

    class DropItem {
       private String objectId;

       public String getObjectId() {
          return objectId;
       }

       public DropItem(String id) {
          this.objectId = id;
       }
    }

    public static void main(String[] args) {
        new DropTest().test();
    }

    public void test() {
        ArrayList<DropItem> allDropsList = new ArrayList<>();       
        allDropsList.add(new DropItem("one"));
        allDropsList.add(new DropItem("two"));
        allDropsList.add(new DropItem("three"));
        allDropsList.add(new DropItem("four"));
        allDropsList.add(new DropItem("five"));

        ArrayList<DropItem> hasReleationList = new ArrayList<>();
        hasReleationList.add(new DropItem("three"));
        hasReleationList.add(new DropItem("five"));

        System.out.println("before:");
        for(DropItem dropItem : allDropsList) {
           System.out.println(dropItem.getObjectId());
        }

        ArrayList<DropItem> result = filterDrops(hasReleationList, allDropsList);
        System.out.println("\n\nafter:");
        for(DropItem dropItem : result) {
           System.out.println(dropItem.getObjectId());
        }
   }

   public ArrayList<DropItem> filterDrops(ArrayList <DropItem> hasRelationList, ArrayList <DropItem> allDropsList) {
       Iterator<DropItem> allDropsIterator = allDropsList.iterator();

       while(allDropsIterator.hasNext()) {
          DropItem dropItemAll = allDropsIterator.next();

          for(DropItem dropItemRelation  : hasRelationList) {
              if(dropItemAll.getObjectId().equals(dropItemRelation.getObjectId())){
                  allDropsIterator.remove();
              }
          }
      }

      return allDropsList;
   }
}

Thanks, I see how your example works, but I am not able to get my design to work this way, I keep getting issues with updating my recyclerView(Possibly because it's waiting for 2 parse queries). Checkout my github listed above. The logic on my method calls on line 90 may be illegal. https://github.com/Hhwhiskey/riple/blob/master/app/src/main/java/com/khfire22gmail/riple/Tabs/TrickleTabFragment.java#L89-L90 — Kevin Hodges, Nov 18 '15 at 00:48
Your methods that load data from Parse are asynchronous (they need to be, since they are doing network calls), but you are treating them as if they are synchronous. In fact, when the methods return (e.g.loadRelationDropsFromParse), the network call will not have completed yet. — GreyBeardedGeek, Nov 18 '15 at 06:20
yes, I actually just figured that out a little while ago. Thanks you for all the help! — Kevin Hodges, Nov 18 '15 at 06:23

Enhanced for loop to remove objects from ArrayList

5 Answers5