Remove elements from ArrayList - Code is skipping elements

Question

What I am trying to do is to loop through a list of indices that refer to a paragraph somewhere. If there paragraph is too short, remove the index from the list.

This is the code I am using at the moment, after googling around, I tried to take the advice to use an Iterator, but still no luck.

    ArrayList<Content> list = anotherList;

    Iterator<Content> i = list.iterator();
    while (i.hasNext()) {

        Content element = i.next();
        int index = list.indexOf(element);

        String paragraph = getContent(index);

        if (paragraph.split(" ").length < minWordCount) {
            i.remove();
        } else {
            irrelevantFuction(index);
        }
    }

The iterator still seems to skip over elements if i.remove() is called, which I understood was exactly what an iterator is meant to prevent.

Could someone point out where I am going wrong?

Thanks

Chris Dennett · Accepted Answer · 2011-04-28 03:28:06Z

3

You should use a ListIterator instead of the normal Iterator if you want access to the index, and keep it updated properly even when elements are removed. Use the listIterator() method to get it and while in the loop, use the nextIndex() to get the index and next() to get the next object, in that order.

Also, don't use the index if you can help it. Just use the object on its own, passing that around as needed, or pass both the index and the element. Either way, don't use the index again for accessing the list.

ListIterator<Content> i = list.listIterator();
int count = 0;
while (i.hasNext()) {
    int index = i.nextIndex();
    Content element = i.next();
       // ...
    }
}

edited Apr 28, 2011 at 3:28

answered Apr 28, 2011 at 1:28

Chris Dennett

22.8k8 gold badges61 silver badges85 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

MByD · Accepted Answer · 2011-04-28 01:19:40Z

1

Don't mix list.indexOf and the iterator. This is what probably causing you problems. Get the string from element (I assume a Content object has the string, right?) and work with it.

while (i.hasNext()) {

    Content element = i.next();
    String paragraph = element.yourMethodToGetTheString() // Some method...

    if (paragraph.split(" ").length < minWordCount) {
        i.remove();
    } else {
        irrelevantFuction(index);
    }
}

answered Apr 28, 2011 at 1:19

MByD

138k30 gold badges269 silver badges278 bronze badges

Comments

Anthony Accioly · Accepted Answer · 2011-04-28 01:34:35Z

1

Is this line int index = list.indexOf(element); returning the correct index (try printing it)? Maybe you should change Content so it can return it's own paragraph String paragraph = element.getParagraph(); And if you are iterating an ArrayList from first to last, why do you need to call indexOf every round?

Try:

    Iterator<Content> i = list.iterator();
    int count = 0;
    while (i.hasNext()) {

        Content element = i.next();

        String paragraph = element.getParagraph();

        if (paragraph.split(" ").length < minWordCount) {
           i.remove();
        } else {
         // fixed according to comment
         int index = count++;
         irrelevantFuction(index);
       }
   }

edited Apr 28, 2011 at 1:34

answered Apr 28, 2011 at 1:22

Anthony Accioly

22.6k9 gold badges76 silver badges125 bronze badges

5 Comments

MByD Over a year ago

your second suggestion is a bit dangerous, incrementing the counter every time while removing elements from the list is exactly his problem.

Anthony Accioly Over a year ago

You're right (In my head he was updating the index or something like that, I will fix the code in a second.

Anthony Accioly Over a year ago

@MByD Done. I think this will reflect correct indexes. @Chris second suggestion is a even better way.

Cornelius Dol Over a year ago

Why not simply irrelevantFunction(count++);?

Anthony Accioly Over a year ago

Yeah, it's equivalent and shorter. But looking at my code I would say that the ListIterator approach is better. I mean, if it's there, use it.

Collectives™ on Stack Overflow

Remove elements from ArrayList - Code is skipping elements

3 Answers 3

Comments

Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related