Check if parameter is duplicated in a list

Question

I've got an issue with my method. I'm supposed to make a Library class in which I add 3 books to a list. That's done. I have to check if the books have the same parameter(ISBN) and I'm having troubles with this part.

public class Library {
    public static void main(String[] args) {

    Book MacMasterRaceVolumeOne = new Book(252510, 2019, "MacMasterRaceVolumeOne");
    Book MacMasterRaceVolumeTwo = new Book(252511, 2020, "MacMasterRaceVolumeTwo");
    Book MacMasterRaceVolumeThree = new Book(252512, 2021, "MacMasterRaceVolumeThree");
    ArrayList<Book> bookList = new ArrayList<>();

    bookList.add(MacMasterRaceVolumeOne);
    bookList.add(MacMasterRaceVolumeTwo);
    bookList.add(MacMasterRaceVolumeThree);
    System.out.println(bookList.);

So far so good. Now my issue comes from the next part checking if the ISBN is duped.

public boolean isISBNMatching(Book bookHere, ArrayList<Book> list) {
    if ((list.contains(bookHere.getISBN() == bookHere.getISBN())){
        return true;
    } 
        return false;
    }
}   

I can see that my idea is flawed, but I think im in the right direction. Cant quite see the correct answer though.

Answer 1

Many problems here.

1. Java convention states that you write getIsbn, and not getISBN.

2. Using an int for ISBN is straight up broken; ints are limited to 9 full digits (they cannot hold any number larger than 2^31-1, which is 10 digits long), and ISBNs have 13. You could use a long, but it is dangerous to use a long for ISBNs: A long is a number and it implies 'math'. It makes absolutely not one iota of sense to add 2 ISBNs together, or to increment an ISBN. That is a strong indicator that long is not appropriate. Most likely, java.lang.String is appropriate, or make a custom type: class Isbn, which holds the data in something encapsulated (a long is more feasible here, you can restrict the code that can even see that long to a few lines and thus make it a lot easier to reason about - in those few lines you probably won't be tempted to consider that long a mathematical concept and e.g. think about adding ISBNs together).

3. In other words, ISBNs should be an object, and you must compare objects with .equals(). Primitives are compared with ==. You are comparing with a single =, which isn't a comparison at all, that is an assignment operator, and that code does not compile.

4. Your list does not contain ISBNs, it contains books. Computer just does what you ask it to and has no brains of its own, thus if you ask it: "Does this list of books contain this ISBN?" the computer will dutifully answer. It will always answer 'no', for the same reason if you ask me if a box of pringles contains a train station, I can truthfully answer: No, it doesn't.

If you must, you CAN loop through the entire list, and for each book in the list, fetch its ISBN and then check that:

public boolean doesListContain(Book book, List<Book> list) {
    for (Book b : list) if (b.getIsbn().equals(book.getIsbn())) return true;
    return false;
}

But, that does mean that as you add more and more books, it becomes slower and slower. If you have a library of a million books and attempt to add a book, your code will have to check all 1 million books to ensure it isn't already there. Not a great solution.

The data structure you are presumably looking for is called a map. Maps map a thing to another thing and can do so in constant time regardless of how many entries it contains.

To visualize this, imagine a phonebook. If I ask you if 'Mr Zaltor' is in there, you will not be manually scanning through every entry in the entire book. Instead, you will open the book to the exact middle entry, see that it reads 'Peterson', and realize that if Zaltor is in here at all, it must be in the bottom half. So, you look at the halfpoint of the bottomhalf (the entry at 75%), and again realize you must look further. Each time to do a lookup you halve the search space. You only need to 'halve the search space' about 20 times to have a definitive answer even if the phone book contains a million entries. That's not quite constant time, but LOG(n) time which is more than good enough and is precisely how e.g. a TreeMap performs. A HashMap is even faster.

"Check if some book with ISBN 'foo' is in this library" is therefore answerable in a way that does not involve manually checking each and every book already inside. Presumably, that's what you want here.

Note: You MUST implement equals and hashCode for your Isbn custom class if you go that route, to use these in a map!

Thus:

Step 1: Make an ISBN class.

class Isbn {
    long code;

    // utility methods to make these and ask about info on them here.

    @Override public boolean equals(Object other) {
        return other instanceof Isbn && ((Isbn) other).code == this.code;
    }

    @Override public int hashCode() {
        return Long.hashCode(code);
    }
}

Step 2: Use a map for your library of books

Map<Isbn, Book> library = new HashMap<>();

// to add a book:

library.put(book.getIsbn(), book);

// To check if an Isbn is already in:

boolean alreadyIn = library.containsKey(book.getIsbn());

// To fetch a book by way of an isbn:

Book b = library.get(someIsbn); // null if not found

// To add a book, but only if not already there.

library.putIfAbsent(book.getIsbn(), book);

Forget about List. It isn't the appropriate data structure for this task.