Problem with removing objects from an ArrayList with objects

Question

On this assignment, I've been given a list of books, but I need to trim down the list of books so only the books that are equal to or less than the max number of pages remain. However, when I try running the code, some of the books are removed properly, but the ones that are above the maximum 400 or 500 pages are not. How can I fix this issue?

Here is the code for the method for the assignment:

public List<Book> filterBooks(List<Book> readingList, int maxPages)
{
    List<Book> correctPages = new ArrayList<Book>();
    for(int i = 0; i < readingList.size(); i++)
    {
        if(readingList.get(i).getNumPages() <= maxPages)
        {
            Book bookMaxPages = readingList.remove(i);
            correctPages.add(bookMaxPages);
        }
    }
    return correctPages;
}

When you remove the index 3 , the number 4 will become index 3. May be can try something like (int i= readinglist.size(); i>=0;i--) — kyorilys, Feb 04 '20 at 02:27

Scary Wombat · Answer 1 · 2020-02-04T02:16:13.417

The main question here is whether you want to modify your original arrayList, if not then

List<Book> correctPages = new ArrayList<Book>();
for(int i = 0; i < readingList.size(); i++)
{
    if(readingList.get(i).getNumPages() <= maxPages)
    {
        // just add
        correctPages.add(readingList.get(i));
    }
}

If you do want to modify, then just reassign correctPages to readingList after calling this method

edit

Suggest you use a for-each loop for readability or see this @mc-emperor java8 answer

MC Emperor · Answer 2 · 2020-02-04T02:30:19.503

The error is because of the fact that you are calling remove directly on your list, while it is being traversed. The removal causes the objects within the readingList to be shifted to the left, but the iterator variable (i) doesn't account for this shift. You need to also update the iterator variable.

An alternative is to use an Iterator<Book>: The iterator's remove() method accounts for the iterator position when an element is removed.

Iterator<Book> it = readingList.iterator();
while (it.hasNext()) {
    if (...) {
        it.remove();
    }
}

You can also solve this using Java 8 streams:

public List<Book> filterBooks(List<Book> readingList, int maxPages) {
    return readingList.stream()
        .filter(book -> book.getNumPages() <= maxPages)
        .collect(Collectors.toList());
}

Furthermore, I strongly suggest not to modify the readingList. The caller may not expect the list to be modified.
Instead, return a new list containing the desired items.

@Abra The *exact* requirements of the assignment are not known. And the sentence should be considered as good advice. — MC Emperor, Feb 04 '20 at 02:28

score 0 · Answer 3 · answered Feb 04 '20 at 02:01

since you already have an answer, I can help point out the error in your code. When you remove an index in the List, the next item replaces it, and when you go to the next iteration, you basically skip one book. Hope that helps and try to take advantage of pass by reference.

score 0 · Answer 4 · answered Feb 04 '20 at 02:53

If you are allowed to use ListIterator interface, then its remove() method can [safely] be used.

ListIterator<Book> lstIter = readingList.listIterator();
while (lstIter.hasNext()) {
    Book book = lstIter.next();
    if (book.getPages() > maxPages) {
        lstIter.remove();
    }
}

After the loop terminates, readingList will contain only books whose number of pages is less than or equal to maxPages.

Note that method remove() in interface ListIterator is defined as an optional operation which means that calling the remove() method may result in a java.lang.UnsupportedOperationException being thrown. Class ArrayList implements interface List and also supports the remove() method. So you could always create an ArrayList from your List like this:

List<Book> correctPages = new ArrayList<>(readingList);
ListIterator<Book> lstiter = correctPages.listIterator();

Problem with removing objects from an ArrayList with objects

4 Answers4