Simple address book in Java

Question

I've answered the following question as best as I could, but I need some help with my design and am wondering if I've taken the correct approach. I would appreciate it if anyone could please point out any mistakes I've made and any improvements I could make.

Question:

Please implement an address book that allows a user to store (between successive runs of the program) the name and phone numbers of their friends, with the following functionality:

• To be able to display the list of friends and their corresponding phone numbers sorted by their name
• Given another address book that may or may not contain the same friends, display the list of friends that are unique to each address book (the union of all the relative complements).

For example given:

Book1 = { "Bob", "Mary", "Jane" }

Book2 = { "Mary", "John", "Jane" }

The friends that are unique to each address book is:

Book1 \ Book2 = { "Bob", "John" }

It is important to provide a solution that highlights your skills in these areas.

• It is also important that your solution highlights your knowledge of and approach to Agile software development.
• The simplest solution is often the best. It is recommended that no more than 4 – 8 hours is spent on the problem, as a sufficient working program can be achieved in that time period.
• The application must run and be easy to build from source. It also must be easy to execute for us to determine if the application meets the above requirements.

Contact Class

import java.io.Serializable;
import java.util.Comparator;

public class Contact implements Serializable {

    private static final long serialVersionUID = 1L;

    private String name;
    private String primaryPhoneNumber;

    public Contact(String name, String primaryPhoneNumber) {
        this.name = name;
        this.primaryPhoneNumber = primaryPhoneNumber;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public String getPrimaryPhoneNumber() {
        return primaryPhoneNumber;
    }

    public void setPrimaryPhoneNumber(String primaryPhoneNumber) {
        this.primaryPhoneNumber = primaryPhoneNumber;
    }

    public String toString() {
        return name + ", " + primaryPhoneNumber;
    }

    public boolean equals(Object obj) {
        if (obj instanceof Contact) {
            Contact contact = (Contact) obj;
            return (name.equals(contact.getName()) && primaryPhoneNumber
                    .equals(contact.getPrimaryPhoneNumber()));
        }

        return false;
    }

    public int hashCode() {
        return (name.length() + primaryPhoneNumber.length());
    }
}

class ContactNameComparator implements Comparator<Contact> {
    public int compare(Contact contact1, Contact contact2) {
        return contact1.getName().compareToIgnoreCase(contact2.getName());
    }
}

Address Book Class

import java.io.Serializable;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

public class AddressBook implements Serializable {

    private static final long serialVersionUID = 1L;

    private String name;
    private List<Contact> contacts;

    public AddressBook(String name) {
        this(name, new ArrayList<Contact>());
    }

    public AddressBook(String name, List<Contact> contacts) {
        this.name = name;
        this.contacts = contacts;
    }

    public void addContact(Contact contact) {
        if (contacts != null) {
            contacts.add(contact);
        }
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public List<Contact> getContacts() {
        return contacts;
    }

    public void setContacts(List<Contact> contacts) {
        this.contacts = contacts;
    }

    public boolean equals(Object obj) {
        if (obj instanceof AddressBook) {
            AddressBook addressBook = (AddressBook) obj;
            return name.equals(addressBook.getName());
        }

        return false;
    }

    public int hashCode() {
        return (name.length());
    }

    public static Set<Contact> getUniqueContacts(List<AddressBook> addressBooks) {

        Set<Contact> commonContacts = new HashSet<Contact>();
        Set<Contact> uniqueContacts = new HashSet<Contact>();

        for (AddressBook addressBook : addressBooks) {
            List<Contact> contacts = addressBook.getContacts();
            List<Contact> allContacts = new ArrayList<Contact>();
            allContacts.addAll(uniqueContacts);
            allContacts.addAll(contacts); 
            contacts.retainAll(uniqueContacts); 
            commonContacts.addAll(contacts); 
            allContacts.removeAll(commonContacts); 

            // set new uinque contacts
            uniqueContacts.clear();
            uniqueContacts.addAll(allContacts);

        }

        return uniqueContacts;

    }
}

AddressBookPersist Class

import java.io.EOFException;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.ArrayList;
import java.util.List;

public class AddressBookPersist {

    private List<AddressBook> addressBooks;

    public AddressBookPersist() {
        addressBooks = new ArrayList<AddressBook>();
        addressBooks = readAddressBooks(); // reads from previous runs      
    }

    public void addAddressBook(AddressBook addressbook) {
        if(!addressBooks.contains(addressbook)){
            addressBooks.add(addressbook);
            storeAddressBooks(addressBooks);
        }

    }

    public void removeAddressBook(AddressBook addressbook) {
        if(addressBooks.contains(addressbook)){
            addressBooks.remove(addressbook);
            storeAddressBooks(addressBooks);
        }

    }

    public List<AddressBook> getAddressBooks() {
        return addressBooks;
    }

    public void setAddressBooks(List<AddressBook> addressBooks) {
        this.addressBooks = addressBooks;
        storeAddressBooks(addressBooks);

    }   

    public void removeAllAddressBooks(){
        addressBooks.clear();
        storeAddressBooks(addressBooks);

    }   

    public void storeAddressBooks(List<AddressBook> addressBooks) {
        try {
            FileOutputStream fos = new FileOutputStream("AddressBooks.txt");
            ObjectOutputStream oos = new ObjectOutputStream(fos);
            oos.writeObject(addressBooks);
            oos.close();
        } catch (IOException e) {
            e.printStackTrace();
        }

    }

    public List<AddressBook> readAddressBooks() {
        List<AddressBook> addressBooks = new ArrayList<AddressBook>();
        try {
            ObjectInputStream ois = new ObjectInputStream(new FileInputStream(
                    "AddressBooks.txt"));
            if(ois.readObject() != null){
                addressBooks = (List<AddressBook>) ois.readObject();
            }
            ois.close();
        } catch (EOFException ex) {
            System.out.println("");
        }catch (FileNotFoundException ex) {
            System.out.println("No address books stored");
        } catch (Exception e) {
            e.printStackTrace();
        }

        return addressBooks;
    }



}

AddressBookTest Class

import static org.junit.Assert.assertTrue;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.junit.Before;
import org.junit.Test;

public class AddressBookTest {
    private Contact c1, c2, c3, c4, c5, c6, c7;
    private AddressBookPersist addressBookPersist;

    @Before
    public void setUp() {
        addressBookPersist = new AddressBookPersist();
        createNewContacts();
    }

    public static void main(String[] args) {
        AddressBookTest test = new AddressBookTest();
        test.setUp();
        test.sortFriendsByTheirNames();
        test.uniqueFriendsFromTwoAddressBooks();
        test.uniqueFriendsFromThreeAddressBooks();
    }

    @Test
    public void uniqueFriendsFromTwoAddressBooks() {
        System.out.println("========== Unique Friends from Two Address Books ==========");

        addTwoAddressBooks();

        List<AddressBook> addressBooks = addressBookPersist.getAddressBooks();
        printInput(addressBooks);

        // Get unique contacts from two AddressBooks
        Set<Contact> uniqueContacts = AddressBook.getUniqueContacts(addressBooks);

        printOutput(addressBooks, uniqueContacts);  

        // The unique contacts from these two address books should be c1 and c4
        Set<Contact> expected = new HashSet<Contact>(Arrays.asList(c1, c4));

        assertTrue(uniqueContacts.equals(expected));
    }

    @Test
    public void uniqueFriendsFromThreeAddressBooks() {
        System.out.println("========== Unique Friends from Three Address Books ==========");

        addThreeAddressBooks();

        List<AddressBook> addressBooks = addressBookPersist.getAddressBooks();
        printInput(addressBooks);       

        // Get unique contacts from three AddressBooks
        Set<Contact> uniqueContacts = AddressBook.getUniqueContacts(addressBooks);

        printOutput(addressBooks, uniqueContacts);

        // The unique contacts from three address books should be c1, c4 and c5, c6, c7
        Set<Contact> expected = new HashSet<Contact>(Arrays.asList(c4, c5, c6, c7));

        assertTrue(uniqueContacts.equals(expected));
    }

    @Test
    public void sortFriendsByTheirNames() {
        System.out.println("========== Display the list of friends sorted by their name ==========");

        AddressBook addressBook = new AddressBook("ab1");
        addressBook.addContact(c5);
        addressBook.addContact(c1);
        addressBook.addContact(c4);
        addressBook.addContact(c2);
        addressBook.addContact(c3);


        System.out.println("==Input==");
        System.out.println("Address Book: " + addressBook.getName());
        System.out.println("Friends:");
        for (Contact contact : addressBook.getContacts()) {
            System.out.println(contact);
        }
        System.out.println();

        Collections
                .sort(addressBook.getContacts(), new ContactNameComparator());
        System.out.println("==Output==");
        System.out.println("Address Book: " + addressBook.getName());
        System.out.println("Friends:");
        for (Contact contact : addressBook.getContacts()) {
            System.out.println(contact);
        }
        System.out.println("\n");

        // Sorted list
        assertTrue("Bob".equals(addressBook.getContacts().get(0).getName()));
        assertTrue("Jane".equals(addressBook.getContacts().get(1).getName()));
        assertTrue("John".equals(addressBook.getContacts().get(2).getName()));
        assertTrue("Mary".equals(addressBook.getContacts().get(3).getName()));
        assertTrue("Ruby".equals(addressBook.getContacts().get(4).getName()));

    }

    private void createNewContacts() {
        c1 = new Contact("Bob", "02 9218 5479");
        c2 = new Contact("Mary", "04 9218 5479");
        c3 = new Contact("Jane", "02 9 605 3147");
        c4 = new Contact("John", "02 605 3147");
        c5 = new Contact("Ruby", "03 9 605 3147");
        c6 = new Contact("Paul", "03 9 605 3147");
        c7 = new Contact("Zee", "03 9 605 3147");

    }

    private void addTwoAddressBooks() {
        addressBookPersist.removeAllAddressBooks();

        AddressBook ab1 = new AddressBook("ab1");
        AddressBook ab2 = new AddressBook("ab2");

        // AddContacts to the addressBooks
        ab1.addContact(c1);
        ab1.addContact(c2);
        ab1.addContact(c3);

        ab2.addContact(c2);
        ab2.addContact(c4);
        ab2.addContact(c3);

        addressBookPersist.addAddressBook(ab1);
        addressBookPersist.addAddressBook(ab2);
    }

    private void addThreeAddressBooks() {
        addressBookPersist.removeAllAddressBooks();

        AddressBook ab1 = new AddressBook("ab1");
        AddressBook ab2 = new AddressBook("ab2");
        AddressBook ab3 = new AddressBook("ab3");

        // AddContacts to the addressBooks
        ab1.addContact(c1);
        ab1.addContact(c2);
        ab1.addContact(c3);

        ab2.addContact(c2);
        ab2.addContact(c4);
        ab2.addContact(c3);

        ab3.addContact(c1);
        ab3.addContact(c5);
        ab3.addContact(c6);
        ab3.addContact(c7);

        addressBookPersist.addAddressBook(ab1);
        addressBookPersist.addAddressBook(ab2);
        addressBookPersist.addAddressBook(ab3);

    }

    private void printInput(List<AddressBook> addressBooks){
        System.out.println("==Input==");

        for (AddressBook addressBook : addressBooks) {
            System.out.println("Address Book: " + addressBook.getName());
            System.out.println("Friends:");
            for (Contact c : addressBook.getContacts()) {
                System.out.println(c.getName());
            }
            System.out.println("");

        }
    }

    private void printOutput(List<AddressBook> addressBooks,Set<Contact> uniqueContacts){
        System.out.println("==Output==");
        System.out.print("Address Books: ");
        String names = "";
        for (AddressBook addressBook : addressBooks) {
            names += addressBook.getName() + ", ";
        }

        if (names.length() > 0) {
            System.out.println(names.substring(0, names.lastIndexOf(",")));
        }
        for (Contact c : uniqueContacts) {
            System.out.println(c.getName());
        }
        System.out.println("\n");
    }

}

Answer 1

Just a few comments:

• Overall design is fine, you have separated the requirements and functions into a correct number of different objects with clear responsibilities, but keeping it simple without overengineering.

• Unit Test class: your main method should not reference the test methods. In fact you don't need a main method at all. You should use the Junit4 test runner. You can run like this: java org.junit.runner.JUnitCore AddressBookTest. This will guarantee to run all your test methods. Generally people are running their tests using Maven or the IDE, so really there is probably not much need to use the command line.

• For return values I prefer to use Collection<T> generally so that client code does not rely on any specific implementation of the data structure. As the code evolves, if I see that client code needs a more specific collection type, then I refactor to return the more specific type. For example: Collection<Contact> getUniqueContacts(List<AddressBook> addressBooks).

• Maybe it simplifies things to have Contact implements Serializable, Comparable, which should be easy since you already built a Comparator.

• I'm not sure I understand the code in getUniqueContacts(). Why not just this:

  public static Set<Contact> getUniqueContacts(List<AddressBook> addressBooks) {
     Set<Contact> unique = new TreeSet<Contact>(); // TreeSet will sort by Contact
     for (AddressBook book : addressBooks) {
        unique.addAll(book);
     }
     return unique;
  }
  

A FEW MORE COMMENTS

I thought of a couple additional basic points you might want to consider:

• add a toString() method to the Contact and Address book classes, which gives a nice String representation of the object. I find I use this more and more, both because it makes it easy to see things in the debugger, and also because it makes logging statements easy.

• Instead of getUniqueContacts, maybe a better method conceptually, is AddressBook merge(List<AddressBook> addressBooks). Taking the point above, and this point, printing unique contacts would look like this:

**

System.out.println("Merged address book=\n" + AddressBook.merge(addressBooksList));

Answer 2

I would use a Set<Contact> in the AddressBook class. This way there cannot be any duplicates. A AddressBook is more like a Set than a List. If you want it sorted by name you can use a TreeSet.

Union, intersection and difference can be implemented quite easy:

Set<Type> union = new HashSet<Type>(s1);
union.addAll(s2);

Set<Type> intersection = new HashSet<Type>(s1);
intersection.retainAll(s2);

Set<Type> difference = new HashSet<Type>(s1);
difference.removeAll(s2);

You can use these to simplify your getUniqueContacts(..) method a lot.

Answer 3

Serializable

Omit Serializable if you do not need it or separate this concern into DTOs

equals / hashcode

Do NOT override hascode and equals if you provide setters. Objects may not be accessable anymore if they were added to a hashcode using structure like the HashSet after you change a value If you usecase specifies to change values in your contact. You can easily see this in this simple example:

public static void main(String[] args) {
    
    Set<Contact> contacts = new HashSet<>();
    
    Contact contact = new Contact("name", "phone");
    
    contacts.add(contact);
    
    System.out.println(contacts.contains(contact)); // returns true

    contact.setName("changed name");
    
    System.out.println(contacts.contains(contact)); // returns false

}

You should always let hashcode and equals methods rely on IMMUTABLE values. In complex environments you will have a hard time to identify such problems. I already faced such a problem in a serialization/deserialization process as not all objects were deserialized again.

Semantic of class "Contact"

That relates to the previous point. Either you do not provide setters for "name" and "phone" OR you should rely on other attributes. I suggest to NOT override equals and hashcode and externalize the equality check into a separate method or class.

"Contact" in business application seems to be a "business object" or a "domain object". Overriding equals and hashcode using all available fields for evaluation will implicitly make an object to a "value object" with "immutable values". Every business object has its own identity NOT depending on attributes under change. They have a unique id. If you have consistency requirements like "unique name" you have to expernalize this check as a "constraint".

Most developers go too fast with hashcode and equals. And if it applicable it is not used. This is because the decision when to rely on hashcode/equals mechanisms is very difficult.

Comparator

Separate the concern of comparing. Prefer Comparator over Comparable as you will be more flexible to add comparison aligorithms that compare your object under a different aspect.