How do I use a for loop inside a for loop? My code executes but does nothing

Question

I am trying to implement a method within a 'Library' class that allows a member to take out an object of Book that has the Title and Author that the user specifies. It first checks to find a book that matches the title&author, and then checks to ensure the member does not already have that book on loan. Here is the code for my method, please let me know where I am going wrong. The borrowBook and if(memberName.getBooks().contains(...) methods both work independently, however when I use them in this block of code they do not seem to execute.

public void memberBorrowBook1(Member memberName, String title2, String author2)
{
    //Test each book object in the library class to see if the parameters match
    for(Book a:books) {
        if(a.getTitle() == title2 && a.getAuthor() == author2); {
            //If title and author match, check to see if the member already has a copy of this book on loan
            for(Book b:memberName.getBooks()) {
                if(memberName.getBooks().contains(a)) {
                    System.out.println("Member already has a copy of that book loaned out");
                //Otherwise, loan the book out to the member
                } else {
                    memberName.borrowBook(a);
                }
            }
        }
    }
}

https://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java — Michael Levy, Dec 14 '17 at 15:58
Start using most powerful features of your idea, i.e. breakpoint — Pratik Shah, Dec 14 '17 at 16:00
Most of the problems in this snippet aren't due to bad String compare, so closing it with the given reason isn't quite right. — laune, Dec 14 '17 at 17:41

score 2 · Answer 1 · answered Dec 14 '17 at 16:08

2

This stands out:

if(a.getTitle() == title2 && a.getAuthor() == author2); {

First: use equals() for comparing String values!

Second: Don't use that semicolon - the if just controls an empty statement (between ) and ; )

Then:

for(Book b:memberName.getBooks()) {   // remove this statement
            if(memberName.getBooks().contains(a)) {

You don't need another loop - the contains does it all.

answered Dec 14 '17 at 16:08

laune

31,114
3
29
42

Awesome thanks, those improvements helped and the code works now. However, if I create 2 objects of book with the same title and author, running this code will result in both books being loaned out to the member - i.e. the if statement to check if the member already owns the book *if(memberName.getBooks().contains(a)) {* doesn't seem to be working.. – swindle Dec 14 '17 at 16:20
Usually books in a library have a _signature_ which identifies the book. So (e.g.) `TreIsl/1` and `TreIsl/2` would be the two copies of _TreasureIsland._ If you have just title and author, you'll have to iterate `books` right to the end to find all sibling copies, and if any of these is contained in `memberName.getBooks()` you know that the member already has it. – laune Dec 14 '17 at 17:38

How do I use a for loop inside a for loop? My code executes but does nothing

1 Answers1