What's wrong with this code? (JAVA)

[hockeygoalie5]

I'm taking a Java tutorial and hit a stone wall when a problem showed up. I checked the code with what the tutorial says, it matches but I get a different result than the tutorial!

MyLibrary.java:

package org.totalbeginner.tutorial;

import java.util.ArrayList;

public class MyLibrary {

    String name;
    ArrayList<Book> books;
    ArrayList<Person> people;

    public MyLibrary(String name) {
        this.name = name;
        books = new ArrayList<Book>();
        people = new ArrayList<Person>();    
    }

    public String getName() {
        return name;
    }

    public ArrayList<Book> getBooks() {
        return books;
    }

    public ArrayList<Person> getPeople() {
        return people;
    }

    public void addBook(Book b1) {
        this.books.add(b1);    
    }

    public void removeBook(Book b1) {
        this.books.remove(b1);
    }
    
    public void addPerson(Person p1) {
        this.people.add(p1);
    }
    
    public void removePerson(Person p1) {
        this.people.remove(p1);
    }

    public boolean checkOut(Book b1, Person p1) {
        int booksOut = this.getBooksForPerson(p1).size();
        if ((b1.getPerson() == null) && 
                (booksOut < p1.getMaximumBooks())) {
            b1.setPerson(p1);
            return true;
        }
        else {
            return false;
        }
    }

    public boolean checkIn(Book b1) {
        if (b1.getPerson() != null) {
            b1.setPerson(null);
            return true;
        }
        else {
            return false;
        }
    }

    public ArrayList<Book> getBooksForPerson(Person p1) {
        ArrayList<Book> result = new ArrayList<Book>();
        for (Book aBook : this.getBooks()) {
            if ((aBook.getPerson() != null) && (aBook.getPerson().getName().equals(p1.getName()))) {
                result.add(aBook);
            }
        }
        return result;
    }
}

MyLibraryTest.java:

package org.totalbeginner.tutorial;

import java.util.ArrayList;

import org.omg.CORBA.PUBLIC_MEMBER;

import junit.framework.TestCase;

public class MyLibraryTest extends TestCase {
    
    private Book b1;
    private Book b2;
    private Person p1;
    private Person p2;
    private MyLibrary ml;

    // test constructor
    public void testMyLibrary() {
        MyLibrary ml = new MyLibrary("Test");
    
        assertEquals("Test", ml.name);
        
        assertTrue(ml.books instanceof ArrayList);
        assertTrue(ml.people instanceof ArrayList);
    }
    
    public void setup() {
        b1 = new Book("Book1");
        b2 = new Book("Book2");
        p1 = new Person();
        p2 = new Person();
        p1.setName("Fred");
        p2.setName("Sue");
        
        ml = new MyLibrary("Test");
    }
    
    public void testAddBook() {
        //create test objects
        setup();
        
        //test initial size is 0
        assertEquals(0, ml.getBooks().size());
        
        ml.addBook(b1);
        ml.addBook(b2);
        
        assertEquals(2, ml.getBooks().size());
        assertEquals(0, ml.getBooks().indexOf(b1));
        assertEquals(1, ml.getBooks().indexOf(b2));
        
        ml.removeBook(b1);
        assertEquals(1, ml.getBooks().size());
        assertEquals(0, ml.getBooks().indexOf(b2));
        
        ml.removeBook(b2);
        assertEquals(0, ml.getBooks().size());
        
    }
    
    public void testCheckOut() {
        // set up objects
        setup();
        
        addItems();
        
        assertTrue("Book did not check out correctly", ml.checkOut(b1,p1));
        
        assertEquals("Fred", b1.getPerson().getName());
        
        assertFalse("Book was already checked out", ml.checkOut(b1,p2));
        
        assertTrue("Book check in failed", ml.checkIn(b1));
        
        
        assertFalse("Book was already checked in", ml.checkIn(b1));
        
        assertFalse("Book was never checked out", ml.checkIn(b1));
        
        // additional test for maximumBooks
        setup();
        addItems();
        
        assertTrue("First book did not check out", ml.checkOut(b2, p1));
        assertFalse("Second book should not have checked out", ml.checkOut(b1, p1));
    }

    private void addItems() {
        ml.addBook(b1);
        ml.addBook(b2);
        ml.addPerson(p1);
        ml.addPerson(p2);
    }
    
    public void testGetBooksForPerson() {
        setup();
        addItems();
        assertEquals(0, ml.getBooksForPerson(p1).size());
        
        ml.checkOut(b1, p1);
        
        ArrayList<Book> testBooks = ml.getBooksForPerson(p1);
        assertEquals(1, testBooks.size());
        assertEquals(0, testBooks.indexOf(b1));
        
        ml.checkOut(b2, p1);
        testBooks = ml.getBooksForPerson(p1);
        assertEquals(2, testBooks.size());
        assertEquals(1, testBooks.indexOf(b2));
    }
}

Test checkOut fails giving the message, "Second book should not have checked out." Passes on tutorial.

[berry120]

Since I can't see your person or book classes it's impossible to tell. But I see no obvious reason why the given test should pass. The checkout method would only return false if the book has already been checked out (which we know not to be the case due to the setup() method resetting everything) or if the person has reached their limit of taking books out (which isn't the case since the book checked out correctly.)

It seems that by default the maximum book limit isn't being set to 1. For that check to work you'll need to make sure it's set as such, either as default on construction or via a method afterwards.

[hockeygoalie5]

Originally Posted by berry120

Since I can't see your person or book classes it's impossible to tell. But I see no obvious reason why the given test should pass. The checkout method would only return false if the book has already been checked out (which we know not to be the case due to the setup() method resetting everything) or if the person has reached their limit of taking books out (which isn't the case since the book checked out correctly.)

It seems that by default the maximum book limit isn't being set to 1. For that check to work you'll need to make sure it's set as such, either as default on construction or via a method afterwards.

If I posted the rest of the project, would it help debug?:
(discluding two listed above and the rest tests)

Person.java:

package org.totalbeginner.tutorial;

public class Person {
    // fields
    private String name; // name of person
    private int maximumBooks; // most books a person can check out 
    
    // constructors
    public Person() {
        name = "unknown name";
        maximumBooks = 3;
    }
    
    // getter
    public String getName() {
        return name;
    }
    
    // setter
    public void setName(String anyName) {
        name = anyName;
    }

    public int getMaximumBooks() {
        return maximumBooks;
    }

    public void setMaximumBooks(int maximumBooks) {
        this.maximumBooks = maximumBooks;
    }
    
    public String toString() {
        return this.getName() + " " + "(" + this.getMaximumBooks() + " " + "books)";
    }
    
}

Book.java:

package org.totalbeginner.tutorial;

public class Book {

     String title;
     String author;
     Person person;

    public Book(String string) {
        this.title = string;
        this.author = "unknown author";
    }

    public String getAuthor() {
        return author;
    }

    public void setAuthor(String author) {
        this.author = author;
    }

    public String getTitle() {
        return title;
    }

    public void setPerson(Person p2) {
        this.person = p2;
        
    }

    public Person getPerson() {
        return this.person;
    }

}

[berry120]

If I posted the rest of the project, would it help debug?:
(discluding two listed above and the rest tests)

Yes - and this just confirms exactly what I said in my previous post! Have a look at your constructor for person, you're setting the maximum number of books to 3. It therefore makes perfect logical sense that the test that fails actually should fail, since the number of books taken out is only 1 when you come to add the second book. The code checks whether you're exceeding the maximum limit, and since 1<3 you're not. It adds the book without question (and rightly so.)

Your actual code is correct, it's the test that's wrong here. The test expects it to not add the book, presumably because it incorrectly assumes maximumBooks is set to 1. It's not - it's set to 3 (since the value isn't changed from the default value in the constructor.)

The long and short of it is you're most likely missing a call to the setMaximumBooks method before your presently failing test case.

[hockeygoalie5]

Looked over the test, found what you were talking about. Thanks for the help!