2
\$\begingroup\$

I have completed the following coding exercise. However, I got poor result for testing. I need some help to find out what is missed, incorrect, or incomplete in the testing section. I would appreciate it if anyone could please point out any mistake in the testing section or possibly any improvement in the related section.

Exercise definition:

I would like an address book application on my PC so that I can keep track of my customer contacts

Acceptance Criteria

  1. Address book will hold name and phone numbers of contact entries
  2. Users should be able to add new contact entries
  3. Users should be able to remove existing contact entries
  4. Users should be able to print all contacts in an address book
  5. Users should be able to maintain multiple address books
  6. Users should be able to print a unique set of all contacts across multiple address books

Written in Java

A working user interface is not required, nor the use of any frameworks.

We are looking for all acceptance criteria to be demonstrable through tests


Contact Class

public class Contact implements Comparable<Contact>{

        private String name;
        private String phoneNumber;

        public Contact(String name, String phoneNumber) {
            setName(name);
            setPhoneNumber(phoneNumber);
        }
    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((name == null) ? 0 : name.hashCode());
        result = prime * result + ((phoneNumber == null) ? 0 : phoneNumber.hashCode());
        return result;
    }

    /**
     * Compares this Contact to the specified object.
     * Two Contacts are equal only if the contact names and phone numbers are equal.
     * Comparison ignores case considerations.
     */
    @Override
    public boolean equals(Object contact) {
        boolean areEquals = false;
        if (contact instanceof Contact) {
            Contact other = (Contact) contact;
            if (other.name.equalsIgnoreCase(name) 
                && other.phoneNumber.equalsIgnoreCase(phoneNumber) ) {
                areEquals = true;
            }
        }
        return areEquals;
    }

    /**
     * Returns a string representation of the Contact. 
     */
    @Override
    public String toString() {
        return "Contact [name=" + name + ", phoneNumber=" + phoneNumber + "]";
    }

    /**
     * Compares two Contacts lexicographically based on the contact name and if the names are equal compares the contact phone numbers.
     * The comparison is based on the Unicode value of each character in the contact name String and contact phone number String.
     * Comparison ignores case considerations.
     * 
     * @return 0 if contacts are equal, greater than 0 if this Contact is greater than specified Contact, and less
     * than 0, otherwise.
     * For Example:
     * returns 0 this Contact("joHN", "04334433") and specified Contact("JOHN", "04334433")
     * returns greater than 0 this Contact("John", "0422435655") and specified Contact("John", "9999999999")
     * returns less than 0 this Contact("John", "9999999999") and specified Contact("John", "0422435655")
     */
    @Override
    public int compareTo(Contact otherContact) {
        final int compareName = name.compareToIgnoreCase(otherContact.getName());
        final int comparePhoneNo = phoneNumber.compareToIgnoreCase(otherContact.getPhoneNumber());
        if (compareName == 0) {
            return comparePhoneNo;
        } else {
            return compareName;
        }
    }

        /** setters and getters **/
}

Constants Class

public final class Constants {


    public static final class Exceptions {
        /**
         * Error message for empty address book
         */
        public final static String EMPTY_ADDRESS_BOOK_ERROR_MSG = "Sorry, address book is not found or it is empty!";

        /**
         * Error message for invalid contact name
         */
        public final static String INVALID_CONTACT_NAME_ERROR_MSG = "Invalid contact name. contact name cannot be null or empty!";

        /**
         * Error message for invalid contact phone number
         */
        public final static String INVALID_CONTACT_PHONE_ERROR_MSG = "Invalid contact phone number. only digits are accepted for phone number!";
    }
}

AddressBook Class

 public class AddressBook {

        List<Contact> contacts;
        private String name;

        public AddressBook(final String name) {
            this.name = name;
            contacts = new ArrayList<Contact>();
        }

        public void addContact(final Contact contact) {
            contacts.add(contact);
        }

        public boolean removeContact(Contact contact) {
            return contacts.remove(contact);    
        }

        public void printContacts() {
            if (getContacts() != null && !getContacts().isEmpty()) {
                System.out.println(this);
            } else {
                throw new EmptyAddressBookException(Constants.Exceptions.EMPTY_ADDRESS_BOOK_ERROR_MSG);
            }
        }

        /** setters, getters, toString **/
    }

SimpleAddressBook Class

public class SimpleAddressBook extends AddressBook implements Serializable {

    private static final long serialVersionUID = 1L;

    /**
     * Constructor calls the parent class constructor.
     * @param name -- simple address book name
     */
    public SimpleAddressBook(final String name) {
        super(name);
    }

}

AddressBookController Class

public class AddressBookController {

    final List<AddressBook> addressBooks;

    public AddressBookController() {
        addressBooks = new ArrayList<AddressBook>();
    }

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

    public void addAddressBook(AddressBook addressBook) {
        addressBooks.add(addressBook);
    }

    public boolean removeAddressBook(AddressBook addressBook) {
        return addressBooks.remove(addressBook);    
    }

    public List<Contact> getAllContacts() {
        final List<Contact> allContacts = new ArrayList<Contact>();
        for (AddressBook addrBook : getAddressBooks()) {
            allContacts.addAll(addrBook.getContacts());
        }
        return allContacts;
    }


    public Set<Contact> getUniqueContacts() {

        Set<Contact> uniqueContacts = new TreeSet<Contact>();
        Set<Contact> commonContacts = new HashSet<Contact>();
        Set<Contact> allContacts = new HashSet<Contact>();
        List<Contact> contacts = null;

        for (AddressBook addrBook : addressBooks) {
            contacts = addrBook.getContacts();

            allContacts.addAll(uniqueContacts);
            allContacts.addAll(contacts);

            contacts.retainAll(uniqueContacts); 
            commonContacts.addAll(contacts);
            allContacts.removeAll(commonContacts); 

            uniqueContacts.clear();
            uniqueContacts.addAll(allContacts);
        }

        return uniqueContacts;
    }

    public void printUniqueContacts() {
        Set<Contact> uniqueContacts = getUniqueContacts();
        Iterator<Contact> itr = uniqueContacts.iterator();
        while (itr.hasNext()) {
            System.out.println(itr.next());
        }
    }

}

Testing Section



AddressBookControllerTest Class

public class AddressBookControllerTest {

    private AddressBookController adController;
    private AddressBook addressBook1;
    private AddressBook addressBook2;
    private Contact contact1;
    private Contact contact2;
    private Contact contact3;
    private Contact contact4;
    private Contact contact5;
    private Contact contact6;


    @Before 
    public void initialize() {
        adController = new AddressBookController();
        addressBook1 = new AddressBook("addrBook1");
        addressBook2 = new AddressBook("addrBook2");
        contact1 = new Contact("John", "0422435655");
        contact2 = new Contact("John", "9999999999");
        contact3 = new Contact("Alex", "0433889977");
        contact4 = new Contact("Simon", "0423888511");
        contact5 = new Contact("Asghar", "0499888");
        contact6 = new Contact("George", "0499888");
    }

    /**
     * Test for adding one address book with multiple contacts to an address book controller.
     */
    @Test
    public void addOneAddressBookTest() {
        final int addrBookSizeBefore = adController.getAddressBooks().size();
        final int contactSizeBefore = addressBook1.getContacts().size();
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact2);
        addressBook1.addContact(contact3);
        addressBook1.addContact(contact4);
        adController.addAddressBook(addressBook1);
        final int addrBookSizeAfter = adController.getAddressBooks().size();
        final int contactSizeAfter = addressBook1.getContacts().size();
        assertEquals(addrBookSizeAfter - addrBookSizeBefore, 1);
        assertEquals(contactSizeAfter - contactSizeBefore, 4);
    }

    /**
     * Test for adding multiple address books with multiple contacts to an address book controller.
     */
    @Test
    public void addMultipleAddressBookTest() {
        final int noAddrBooksBefore = adController.getAddressBooks().size();
        final int noContactsAddr1Before = addressBook1.getContacts().size();
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact2);
        adController.addAddressBook(addressBook1);

        final int noContactsAddr2Before = addressBook2.getContacts().size();
        addressBook2.addContact(contact3);
        addressBook2.addContact(contact4);
        adController.addAddressBook(addressBook2);

        final int noAddrBooksAfter = adController.getAddressBooks().size();
        final int noContactsAddr1After = addressBook1.getContacts().size();
        final int noContactsAddr2After = addressBook2.getContacts().size();

        assertEquals(noAddrBooksAfter - noAddrBooksBefore, 2);
        assertEquals(noContactsAddr1After - noContactsAddr1Before, 2);
        assertEquals(noContactsAddr2After - noContactsAddr2Before, 2);
    }

    /**
     * Test for retrieving all the contacts of all address book from address book controller.
     * Duplicate contact is allowed in this list.
     */
    @Test
    public void getAllContactsTest() {
        final int noContactsBefore = adController.getAllContacts().size();
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact3);
        addressBook1.addContact(contact1);

        addressBook2.addContact(contact1);
        addressBook2.addContact(contact2);
        addressBook2.addContact(contact4);

        adController.addAddressBook(addressBook1);
        adController.addAddressBook(addressBook2);

        final int noContactsAfter = adController.getAllContacts().size();
        assertEquals(noContactsAfter - noContactsBefore, 6);
    }

    /**
     * Test for removing an address books with multiple contacts from address book controller.
     */
    @Test
    public void removeAddressBookTest() {
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact3);

        addressBook2.addContact(contact2);
        addressBook2.addContact(contact4);

        adController.addAddressBook(addressBook1);
        adController.addAddressBook(addressBook2);

        final int addrBookSizeBefore = adController.getAddressBooks().size();
        final int contactSizeBefore = adController.getAllContacts().size();
        adController.removeAddressBook(addressBook1);
        final int addrBookSizeAfter = adController.getAddressBooks().size();
        final int contactSizeAfter = adController.getAllContacts().size();

        assertEquals(addrBookSizeAfter - addrBookSizeBefore, -1);
        assertEquals(contactSizeAfter - contactSizeBefore, -2);
    }

    /**
     * Test for retrieving unique contacts of all address books from address book controller.
     */
    @Test
    public void getUniqueContactsAcrossAddrBooksTest() {

        addressBook1.addContact(contact1);
        addressBook1.addContact(contact3);
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact5);
        addressBook1.addContact(contact6);

        addressBook2.addContact(contact1);
        addressBook2.addContact(contact2);
        addressBook2.addContact(contact4);
        addressBook2.addContact(contact6);

        adController.addAddressBook(addressBook1);
        adController.addAddressBook(addressBook2);

        // expected unique contacts
        List<Contact> expectedUniqueContacts = new ArrayList<Contact>();
        expectedUniqueContacts.add(contact3);
        expectedUniqueContacts.add(contact2);
        expectedUniqueContacts.add(contact4);
        expectedUniqueContacts.add(contact5);

        // expected common contacts
        List<Contact> expectedCommonContacts = new ArrayList<Contact>();
        expectedCommonContacts.add(contact1);
        expectedCommonContacts.add(contact6);

        // getting all unique contacts
        Set<Contact> uniqueContacts = adController.getUniqueContacts();

        // all the expected unique contacts should be included in the output unique contact list.
        for (Contact contact : expectedUniqueContacts) {
            assertTrue(uniqueContacts.contains(contact));
        }
        // none of the excepted common contacts should be in the output unique contact list.
        for (Contact contact : expectedCommonContacts) {
            assertFalse(uniqueContacts.contains(contact));
        }
    }

    /**
     * Test for printing unique contacts of all address books in address book controller.
     */
    @Test
    public void printUniqueContactsAcrossAddrBooksTest() {
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact3);
        addressBook1.addContact(contact1);
        addressBook1.addContact(contact5);
        addressBook1.addContact(contact6);

        addressBook2.addContact(contact1);
        addressBook2.addContact(contact2);
        addressBook2.addContact(contact4);
        addressBook2.addContact(contact6);

        adController.addAddressBook(addressBook1);
        adController.addAddressBook(addressBook2);
        adController.printUniqueContacts();
    }

}

AddContactTest Class

public class AddContactTest {

    /**
     * Test 1 for invalid contact with empty contact name
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactName1() {
        new Contact("", "0422435655");
    }

    /**
     * Test 2 for invalid contact with null contact name
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactName2() {
        new Contact(null, "9999999999");
    }

    /**
     * Test 3 for invalid contact with null contact phone number
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactPhoneNo1() {
        new Contact("Simon", null);
    }

    /**
     * Test 4 for invalid contact with empty contact phone number
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactPhoneNo2() {
        new Contact("Simon", "");
    }

    /**
     * Test 5 for invalid contact with a contact phone number having non-digit characters
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactPhoneNo3() {
        new Contact("Alex", "043C8+9977");
    }

    /**
     * Test 6 for invalid contact with a contact phone number having non-digit characters
     */
    @Test(expected=IllegalArgumentException.class)
    public void testInvalidContactPhoneNo4() {
        new Contact("Simon", "9877c9876");
    }

    /**
     * Test 7 for valid contact with valid contact names and phone numbers
     */
    @Test
    public void testValidContacts() {
        Contact contact = new Contact("Simon", "9");
        assertEquals(contact.getName(), "Simon");
        assertEquals(contact.getPhoneNumber(), "9");

        contact = new Contact("Sn", "9980980");
        assertEquals(contact.getName(), "Sn");
        assertEquals(contact.getPhoneNumber(), "9980980");
    }
}

AddressBookAddContactTest Class

public class AddressBookAddContactTest {

    private Contact contact1;
    private Contact contact2;
    private Contact contact3;
    /**
     * Address book
     */
    private AddressBook addressBook;
    /**
     * Contact list size before calling a specific method.
     */
    private int contactSizeBefore;
    /**
     * Contact list size after calling a specific method.
     */
    private int contactSizeAfter;

    /**
     * Initializing new contacts and an address book
     */
    @Before 
    public void initialize() {
        contact1 = new Contact("John", "0422435655");
        contact2 = new Contact("John", "9999999999");
        contact3 = new Contact("Alex", "0433889977");
        addressBook = new SimpleAddressBook("addrBook1");
    }

    /**
     * Test add zero contact to address book
     */
    @Test
    public void testAddZeroContactToAddressBook() {
        assertEquals(addressBook.getContacts().size(), 0);  
    }

    /**
     * Test add one contact to address book
     */
    @Test
    public void testAddOneContactToAddressBook() {
        contactSizeBefore = addressBook.getContacts().size();
        addressBook.addContact(contact1);
        contactSizeAfter = addressBook.getContacts().size();
        assertEquals(contactSizeAfter - contactSizeBefore, 1);

        Iterator<Contact> contactIt = addressBook.getContacts().iterator();
        while (contactIt.hasNext()) {
            assertEquals(contactIt.next(), contact1);
        }   
    }

    /**
     * Test add multiple contacts to address book
     */
    @Test
    public void testAddMultipleContactsToAddressBook() {
        contactSizeBefore = addressBook.getContacts().size();
        addressBook.addContact(contact1);
        addressBook.addContact(contact2);
        addressBook.addContact(contact3);
        contactSizeAfter = addressBook.getContacts().size();
        assertEquals(contactSizeAfter - contactSizeBefore, 3);  
    }

    /**
     * Test add multiple contacts with duplicates to address book
     */
    @Test
    public void testAddMultipleContactsWithDuplicateToAddressBook() {
        // contact list accepts duplicate contact
        contactSizeBefore = addressBook.getContacts().size();
        addressBook.addContact(contact1);
        addressBook.addContact(contact1);
        contactSizeAfter = addressBook.getContacts().size();
        assertEquals(contactSizeAfter - contactSizeBefore, 2);  
    }

    /**
     * Test add multiple contacts without duplicates to address book
     */
    @Test
    public void testAddContactsWithoutDuplicateToAddressBook() {    
        // contact list accepts duplicate contact
        contactSizeBefore = addressBook.getContacts().size();
        addressBook.addContact(contact1);
        addressBook.addContact(contact2);
        contactSizeAfter = addressBook.getContacts().size();
        assertEquals(contactSizeAfter - contactSizeBefore, 2);
    }
}

AddressBookPrintContactTest Class

public class AddressBookPrintContactTest {

    /**
     * Address book
     */
    private AddressBook addressBook;

    /**
     * Initializing an empty address book
     */
    @Before 
    public void initialize() {
        addressBook = new SimpleAddressBook("addrBook1");
    }

    /**
     * Test expects EmptyAddressBookException when printing an empty address book.
     */
    @Test(expected=EmptyAddressBookException.class)
    public void testPrintContacts() {
        addressBook.printContacts();
    }

}

AddressBookRemoveContactTest Class

public class AddressBookRemoveContactTest {

    private Contact contact1;
    private Contact contact2;
    private Contact contact3;
    /**
     * Address book
     */
    private AddressBook addressBook;

    /**
     * Initializing few contacts and a simple address book
     */
    @Before 
    public void initialize() {
        contact1 = new Contact("John", "0422435655");
        contact2 = new Contact("John", "9999999999");
        contact3 = new Contact("Nic", "049982845");
        addressBook = new SimpleAddressBook("addrBook1");
    }

    /**
     * Test for removing none-existent contact from address book
     */
    @Test
    public void testRemoveNonExistentContactFromAddressBook() {
        assertFalse(addressBook.getContacts().contains(contact3));
        assertFalse(addressBook.removeContact(contact3));
        assertFalse(addressBook.getContacts().contains(contact3));  
    }

    /**
     * Test for removing a contact from address book
     */
    @Test
    public void testRemoveContactFromAddressBook() {
        addressBook.addContact(contact1);
        addressBook.addContact(contact2);
        addressBook.removeContact(contact1);
        assertFalse(addressBook.getContacts().contains(contact1));
    }
}

CompareContactTest Class

public class CompareContactTest {

    private Contact contact1;
    private Contact contact2;
    private Contact contact3;
    private Contact contact4;
    private Contact contact5;

    /**
     * Initializing few contacts
     */
    @Before 
    public void initialize() {
        contact1 = new Contact("John", "0422435655");
        contact2 = new Contact("JOHN", "0422435655");
        contact3 = new Contact("John", "9999999999");
        contact4 = new Contact("Alex", "0433889977");
        contact5 = new Contact("Simon", "0443889977");
    }

    /**
     * Test for equals method of Contact class. Two Contacts are equal only if the contact names and phone numbers are equal.
     */
    @Test
    public void testEqualContacts() {
        assertTrue(contact1.equals(contact2));
        assertTrue(contact2.equals(contact1));
        assertFalse(contact1.equals(contact3));
        assertFalse(contact3.equals(contact1));
        assertFalse(contact4.equals(contact5));
    }
    /**
     * Test for compareTo method of Contact class. 
     * compareTo returns 0 if contacts are equal, greater than 0 if this Contact is greater than specified Contact, and less.
     */
    @Test
    public void testCompareContacts() {
        assertTrue(contact1.compareTo(contact2) == 0);
        assertTrue(contact2.compareTo(contact1) == 0);
        assertTrue(contact1.compareTo(contact3) < 0);
        assertTrue(contact3.compareTo(contact1) > 0);
        assertTrue(contact4.compareTo(contact5) < 0);
    }

}
\$\endgroup\$
  • \$\begingroup\$ Why not use code coverage of your IDE of choice to see for yourself? While code coverage does not give you all the answerers, it can hint at cases you have overlooked. \$\endgroup\$ – hotzst Jan 5 '17 at 21:42
1
\$\begingroup\$

1) Not enough Code in Contact class for Tests (validation and compareTo)

2)

I got poor result for testing

That should be better explained! Right now i have failed tests for comparator (CompareContactTest.class) and validation (AddContactTest.class), because of missing code.

3) Explain pls the model. i see you allow duplicated contacts. Why? What a difference between unique, common and all contacts? And so on...

REFACTORING:

AddressBook:

Why should it be a problem, that address book is empty? Just empty output would be ok in my opinion.

 public void printContacts() {
            if (getContacts() != null && !getContacts().isEmpty()) {
                System.out.println(this);
            } else {
                throw new EmptyAddressBookException(Constants.Exceptions.EMPTY_ADDRESS_BOOK_ERROR_MSG);
            }
        }

I would define all containers as final. For Example AddressBook:

 final List<Contact> contacts = new ArrayList<>();
    private String name;

    public AddressBook(final String name) {
        this.name = name;       
    }

    public void printContacts() {
        for (final Contact c : this.contacts){
            System.out.println(c);
        }
    }

AddressBookController:

 public void printUniqueContacts() {
        Set<Contact> uniqueContacts = getUniqueContacts();
        Iterator<Contact> itr = uniqueContacts.iterator();
        while (itr.hasNext()) {
            System.out.println(itr.next());
        }
    }

it can be replaced with one line if you use java 8:

public void printUniqueContacts() {
      getUniqueContacts().stream().forEach(c -> System.out.println(c));
}

---------------------

Add toString method to Contact and AddressBook. Otherwise you see only object references.

TESTING: Looks good. Each function is tested. I would not create so many test classes und use one test class pro one class. But it is a matter of taste.

What not good - that is there is no normal message if test fails. The better style would be:

  assertEquals("Contact had a wrong phone number!", contact.getPhoneNumber(), "9980980");

Or sometimes ll be needed even something like this:

 @Test
public void testInvalidContactName1() {        
    try {
        new Contact("", "0422435655");
    } catch (final IllegalArgumentException ex){            
        fail("The name should not be empty!");
    }
}

But the above example is not too good. If you just forget to throw your exception in code, this example will not fail. The better way would be:

   @Test
public void testInvalidContactName1() {
    boolean hasRaised = false;
    try {
        new Contact("", "0422435655");
    } catch (final IllegalArgumentException ex){
        hasRaised = true;
    }
    assertTrue("The name should not be empty!", hasRaised);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.