Method to add unique object instance into an ArrayList is not working

Question

First of all: I am new to Java and this site, this is my first post here. I have searched on both Google and this site regarding my question and found some (a lot) of similar posts, but I can't found one that helps me with my coding problem.

I have two classes: Library and Book. For the last four hours, I have been trying to write a method that adds a book to the Library (to an ArrayList) if it's unique. If it's not unique, a message will be printed saying that the Library already has the book.

Class: Library

Field:

private ArrayList<Book> bookList;

Constructor:

bookList = new ArrayList<Book>();

Method:

public void addBook(Book bookTitle)
{
for(Book bookId:bookList) {
if (bookTitle.equals(bookId)) {
bookList.add(bookTitle);
System.out.println("Book registered."); }
else { System.out.println("This book is not unique"; }                
}

}

I do not get any compilation error but it's not working. The object is not added to my ArrayList, no message is printed. I have also tried with:

if (bookId != bookTitle)

without result.

Hope someone here can help and explain to me what I am doing wrong, it would be greatly appreciated.

You might have missed overriding `equals()` method in the class, `Book`. Check [Why do I need to override the equals and hashCode methods in Java?](https://stackoverflow.com/questions/2265503/why-do-i-need-to-override-the-equals-and-hashcode-methods-in-java)? — Arvind Kumar Avinash, Nov 11 '20 at 19:54

Rob Evans · Accepted Answer · 2020-11-11T20:18:36.760

Seems you got confused with what the expression you're evaluating in the IF statement is doing. You're if expression effectively says: "if bookTitle matches one in bookList (ie. is a duplicate)" Do X, else Y.

The "do X" branch says "Add the book" booklist.add(bookTitle) - when its a duplicate - which is wrong.

The "do Y" branch of your if says (corrected syntax): System.out.println("This book is not unique"); which has no effect on the bookList

Instead I've swapped the two branches X and Y -> Y and X So you should have:

    public void addBook(Book bookTitle){
      for(Book bookId : bookList) {
        if (bookTitle.equals(bookId)) {
          System.out.println("This book is not unique");
          break; // breaks out of the surrounding for-loop
          // stop checking books if we enter this branch
        } else {
          bookList.add(bookTitle);
          System.out.println("Book registered."); 
          // keep checking more books if we enter this branch
        }                
      } 
    }

Research tells us the more nested our code is the more likely it is for bugs to occur as the logic is harder to digest.

I've had a go at breaking it all apart and came up with:

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

class LibraryApplication {

    static Library library;

    public static void main(String[] args) {
        library = new Library();
        addBook(new Book("Title 1"));
        addBook(new Book("Title 1"));
        addBook(new Book("Title 2"));
        library.printAllBooks();
    }

    public static void addBook(Book bookTitle){
        if (library.isBookInLibrary(bookTitle)) {
            System.out.println("This book (" + bookTitle.getTitle() + ") is not unique");
        } else {
            library.add(bookTitle);
            System.out.println("Book " + bookTitle.getTitle() + " registered.");
        }
    }
}

class Book {

    private String title;

    public Book(String title) {
        this.title = title;
    }

    public String getTitle() {
        return title;
    }

    // Called by Library's printAllBooks method (library.toString())
    @Override
    public String toString() {
        return "Book{" +
                "title='" + title + '\'' +
                '}';
    }

    // auto generated by IDE (important for checking whether a collection contains an equal object)
    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof Book)) return false;
        Book book = (Book) o;
        return title.equals(book.title);
    }

    // auto generated by IDE (important for checking whether a collection contains an equal object)
    @Override
    public int hashCode() {
        return Objects.hash(title);
    }

}

class Library {
    List<Book> library = new ArrayList<Book>();

    public boolean isBookInLibrary(Book book){
        return library.contains(book);
    }

    public void add(Book book) {
        library.add(book);
    }

    public void printAllBooks() {
        System.out.println(library.toString());
    }
}

Output:

Book Title 1 registered.
This book (Title 1) is not unique
Book Title 2 registered.
[Book{title='Title 1'}, Book{title='Title 2'}]


Process finished with exit code 0

Method to add unique object instance into an ArrayList is not working

1 Answers1