Return to Answer

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);
}

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");

1. Not enough Code in Contact class for Tests.

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.

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

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.