Removing items from a list in Java

user_10803017 0
Viewed 1832

In my computer science class, we use an online program called CodeHS that gives us assignments that cover certain topics. Usually, I understand ArrayLists relatively well, but this assignment uses Lists instead and it is kind of tripping me up. I have been struggling a little bit and my code (I will link it at the bottom) is returning the exact list that they insert in the method parameters. (in other words, my code is not doing what I want it to. It's not doing anything at all.) I am very new to this and would appreciate some help, with helpful criticism rather than scolding, haha. Thank you.

Here is the Assignment:

You’ve been given a list of books to read over the summer, but you need to trim down the list of books so you can finish all of them.

Write a method

public List<Book> filterBooks(List<Book> readingList, int maxPages)

That takes a List of Books as a parameter, removes all Books from the readingList that have more than maxPages pages, then returns the resulting list.

You can access the number of pages of a Book by calling book.getNumPages(). The Book class is provided for reference. 

public class Book
{
private String title;
private String author;
private int numPages;

public Book(String theTitle, String theAuthor, int numberOfPages)
{
    title = theTitle;
    author = theAuthor;
    numPages = numberOfPages;
}

public String getTitle()
{
    return title;
}

public String getAuthor()
{
    return author;
}

public int getNumPages()
{
    return numPages;
}

public String toString()
{
    return title + " by " + author;
}

public boolean equals(Book other)
{
    return title.equals(other.title) && author.equals(other.author);
}
}

This is what I have tried:

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

    return readingList;

}
arraylist java
4 Answers

You could use the List.removeIf(...) function, (if it's supported by the list) that does exactly what you want: remove all objects in that list if they meet a criteria.

So the code would be (with lambda):

readingList.removeIf(b -> b.getNumPages() <= maxPages);

Or without lambdas:

readingList.removeIf(new Predicate<Book>() {
    @Override
    public boolean test(Book b) {
        return b.getNumPages() <= maxPages;
    }
}
user_71504080

Your code does not appear to be what you described. You should be removing some items from the original list, so it shouldn't return the exact same list, unless none of the books you checked matched the criteria for removal. (Assuming the list passed in is mutable.)

However you also have a couple bugs. Based on the problem description your condition should be > not >=. You also have a bug in the way you don't compensate for removing from the original list when you address the elements by index. If you remove an item at index i you should not increment i because the next book will now be in that position and you don't want to skip it. You will skip some books that you should have eliminated if there are two books in a row that should be removed.

user_11657240

Can you use Java8? Use lambdas then.

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

If not - never modify the list while you are iterating it. Create a new one instead.

public List<Book> filterBooks(List<Book> readingList, int maxPages) {
    List<Book> result = new ArrayList<>();
    for (Book book : readingList){
        if (book.getNumPages() <= maxPages){
            result.add(book);
        }
    }
    return result;
}

Always try using abstractions (e.g. List) instead of real implementations (e.g. ArrayList) in declarations. It will allow you to change implementation in the future and be more flexible.

Please refer to Liskov Principle - one of the SOLID principles.

user_12665000

You created a new list readingList2, but you are not using it anywhere. You are trying to change the list which you accepted as a formal parameter.

Why not approach the problem a bit differently and return a new list with only the values you need?

Code Snippet:

public List<Book> filterBooks(List<Book> readingList, int maxPages) {
    List<Book> newReadingList = new ArrayList<>();
    for(final Book currentBook : readingList) {
        if(currentBook.getNumPages() < maxPages) {
           newReadingList.add(currentBook);
        }
    }
    return newReadingList;
}
user_20046850
Related