I can only access first item in the list

Question

public class Catalogue() {
  private List<Book> booksAvailable;
  private List<Book> booksRented:

  public Catalogue() {
    booksAvailable.add(new Book("Matrix", 1999, new Genre("SciFi"), 3));
    booksAvailable.add(new Book("Jurassic Park", 1999, new Genre("SciFi"), 3));
    boosAvailable.add(new Book("Terminator", 1999, new Genre("SciFi"), 3));
    booksRented = new LinkedList<Book> ();
  }

  public void rentBook() {
    System.out.println("Rent a book:");
    System.out.println("Enter the title of a book you want to rent: ");
    String name = In.NextLine();
    for (Book book: booksAvailable) {
      if (book.getName.equals(name)) {
        System.out.println("Renting " + name);
        booksAvailable.remove(book);
        booksRented.add(book);
        break;
      } else {
        System.out.println("No such books found");
      }
    }
  }
}

While running this code can only rent the Matrix book. When I try to rent another book like Jurassic park it says that no books found. When I close the program and again run it and try to rent the second book then it again says the books not found. Please help me with this problem. What is the problem that i have in this code. Thanks

Also, you need to include the code that is failing. When you're comparing strings it is easy to include a type-o. — matt, May 04 '18 at 10:58
I still don't understand how do you add books on an uninitialised `booksAvailable` list. — drgPP, May 04 '18 at 11:00
Possible duplicate of [Calling remove in foreach loop in Java](https://stackoverflow.com/questions/1196586/calling-remove-in-foreach-loop-in-java) — AxelH, May 04 '18 at 11:03
@matt When I run this code I can see there some updates on the booksAvailable list but I won't see any of the movies added on the booksRented list. It only shows [ ] as a output. — Pramish Luitel, May 04 '18 at 11:23

score 1 · Answer 1 · answered May 04 '18 at 11:27

As others have pointer out modifying a list while you're iterating over it is dangerous.

I would recommend trying it with a HashMap instead, especially if name is the only field you're looking at.

public class Catalogue {
    private Map<String, Book> booksAvailable;
    private Map<String, Book> booksRented;

    public Catalogue() {
        booksAvailable = new HashMap<>();
        booksAvailable.put("Matrix", new Book("Matrix", 1999, new Genre("SciFi"), 3));
        booksAvailable.put("Jurassic Park", new Book("Jurassic Park", 1999, new Genre("SciFi"), 3));
        booksAvailable.put("Terminator", new Book("Terminator", 1999, new Genre("SciFi"), 3));
        booksRented = new HashMap<>();
    }

    public void rentBook() {
        System.out.println("Rent a book:");
        System.out.println("Enter the title of a book you want to rent: ");
        Scanner scanner = new Scanner(System.in);
        String name = scanner.nextLine();
        if (booksAvailable.containsKey(name)) {
            Book book = booksAvailable.get(name);
            System.out.println("Renting " + name);
            booksAvailable.remove(name);
            booksRented.put(name, book);
        } else {
            System.out.println("No such books found");
        }
    }
}

Modifying a list while iterating over it with the for each loop will cause a concurrent modification exception. — matt, May 04 '18 at 12:03

Mạnh Quyết Nguyễn · Answer 2 · 2018-05-04T11:38:53.740

0

When you call booksAvailable.remove() it will effectively removed from current iteration.

Hence when you access next(), it might result in unexpected behavior.

Edit You cannot rent other book other than first book because of your code handling.

You have System.out.println("No such books found"); in the else statement inside loop. So if you input a book other than the first book, the test fail and the statement is printed.

To correct this you can use a flag to indicate a book is found or not, and print the statement outside the loop;

boolean rented = false;
for (Book b : books) {
    if (found) {
       rented = true;
    }
}
if (!rented) {
   // print no such book message
}

edited May 04 '18 at 11:38

answered May 04 '18 at 10:58

Mạnh Quyết Nguyễn

17,677
1
23
51

They are not using an iterator. – matt May 04 '18 at 11:00
5

@matt The enhanced for loop `for (Book book: booksAvailable)` uses `Iterator` internally. – Zabuzard May 04 '18 at 11:02
@Zabuza right and it is not exposed, and hence you cannot use booksAvailable.remove() because that is a method on the Iterator, not on the list itself. Same with next. – matt May 04 '18 at 11:03
I can see the other books which are on the available list but I won't see on the rented list. But when I try to rent other books rather than the first book then is shows no books available. – Pramish Luitel May 04 '18 at 11:27

score 0 · Accepted Answer · answered May 04 '18 at 11:27

0

I just tried your code(after modifying it to compile) and it works. However when running on for example Jurasic Park it will say that the book is not found and then it will rent it, because that print statement is in the for loop.

I tried rewriting it to use streams and optionals, and I got this code that seems to be working

public void rentBook() {
    System.out.println("Rent a book:");
    System.out.println("Enter the title of a book you want to rent: ");
    String name = "Jurassic Park";
    Optional<Book> book = booksAvailable.stream().filter(b -> b.name.equals(name)).findFirst();
    if(book.isPresent()) {
        System.out.println("Renting " + name);
        booksAvailable.remove(book.get());
        booksRented.add(book.get());
    }
    else
        System.out.println("No such book found");
}

answered May 04 '18 at 11:27

munHunger

2,572
5
34
63

hey thanks for the code but when I tried to print book.toString() then it shows the "Optional[1999 Matrix SciFi $3] removed from catalogue." – Pramish Luitel May 04 '18 at 12:17
@PramishLuitel Yeah, that is because in the code I sent you the book is wrapped in an `Optional` so you need to unwrap it with `book.get()` which will give you the book object. So what you want is `book.get().toString()`... http://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html I would highly recommend reading that if you have time over since the `Optional` style of coding is rather neat :) – munHunger May 04 '18 at 12:28
Thanks @munHunger – Pramish Luitel May 04 '18 at 12:36

I can only access first item in the list

3 Answers3