ContactsList implementation in Java

Question

I created a ContactsList class in Java that holds Person objects. I consider this is a side project for my GitHub profile. Any kind of suggestion is appreciated.

Person Class

package contactsList; public class Person { private static final String FAVOURITE = "★"; private String firstName; private String lastName; private String fullName; private String phone; private String dob = ""; private String email = ""; private String favourite = ""; public Person(String firstName, String lastName, String phone) { setFirstName(firstName); setLastName(lastName); setFullName(this.getFirstName().concat(" ").concat(this.getLastName())); setPhone(phone); } public Person(String firstName, String lastName, String phone, String dob, String email) { this(firstName, lastName, phone, dob, email, false); } public Person(String firstName, String lastName, String phone, String dob, String email, boolean favourite) { this(firstName, lastName, phone); this.setDob(dob); this.setEmail(email); this.setFavourite(favourite); } public boolean getFavourite() { if(this.favourite.equals(FAVOURITE)) return true; else return false; } public void setFavourite(boolean favourite) { if(favourite) this.favourite = FAVOURITE; } public String getFullName() { return fullName; } public void setFullName(String fullName) { this.fullName = fullName; } public String getFirstName() { return firstName; } public void setFirstName(String firstName) { this.firstName = firstName; } public String getLastName() { return lastName; } public void setLastName(String lastName) { this.lastName = lastName; } public String getPhone() { return phone; } public void setPhone(String phone) { this.phone = phone; } public String getDob() { return dob; } public void setDob(String dob) { this.dob = dob; } public String getEmail() { return email; } public void setEmail(String email) { this.email = email; } @Override public String toString() { return "Person{" + "firstName='" + firstName + '\'' + ", lastName='" + lastName + '\'' + ", phone='" + phone + '\'' + ", dob='" + dob + '\'' + ", email='" + email + '\'' + ", favourite='" + favourite + '\'' + '}'; } } 

ContactsList Class

package contactsList; import java.util.List; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.Iterator; import static java.lang.System.out; public class ContactsList implements Iterable<Person> { private List<Person> personList = new ArrayList <>(); private Person person; public ContactsList() {} public ContactsList(Person person) { this(person, false); } public ContactsList(Person person, boolean favourite) { this.person = person; this.person.setFavourite(favourite); personList.add(this.person); } public void addContacts(Person person) { personList.add(person); } public void addContacts(String firstName, String lastName, String phone) { this.addContacts(firstName, lastName, phone, "", "", false); } public void addContacts(String firstName, String lastName, String phone, String dob, String email) { this.addContacts(firstName, lastName, phone, dob, email, false); } public void addContacts(String firstName, String lastName, String phone, String dob, String email, boolean favourite) { this.person = new Person(firstName, lastName, phone, dob, email, favourite); this.personList.add(this.person); } public int getTotalContacts() { return personList.size(); } public boolean exists(Person person) { if(personList.contains(person)) return true; else return false; } public boolean exists(String fullName) { for(Person person : personList) { if(person.getFullName().equals(fullName)) return true; } return false; } public void removeContacts(Person person) { if(personList.remove(person)) return; } public void removeContacts(String fullName) { int index = -1; for(Person person : personList) { if(person.getFullName().equals(fullName)) index = personList.indexOf(person); } if(index >= 0) personList.remove(index); } public void displayFavouriteContacts() { for(Person person : personList) { if(person.getFavourite()) out.println(person); } } public void sortBy(String by) { final String f = "f"; final String l = "l"; if(by.equalsIgnoreCase(f)) Collections.sort(personList, Comparator.comparing(Person::getFirstName)); else if(by.equalsIgnoreCase(l)) Collections.sort(personList, Comparator.comparing(Person::getLastName)); else Collections.sort(personList, Comparator.comparing(Person::getFullName)); } public List<String> getPhoneNumber(String fullName) { List<String> phoneList = new ArrayList <>(); for(Person person : personList) { if(person.getFullName().equals(fullName)) phoneList.add(person.getPhone()); } return phoneList; } public List<String> getPhoneByFirstName(String firstName) { List<String> phoneList = new ArrayList <>(); for(Person person : personList) { if(person.getFirstName().equals(firstName)) phoneList.add(person.getPhone()); } return phoneList; } public List<String> getPhoneByLastName(String lastName) { List<String> phoneList = new ArrayList <>(); for(Person person : personList) { if(person.getLastName().equals(lastName)) phoneList.add(person.getPhone()); } return phoneList; } public List<Person> getAllContacts() { List<Person> tempList = new ArrayList <>(); for(Person person : personList) tempList.add(person); return tempList; } @Override public String toString() { String longString = ""; for(Person person : personList) longString = longString.concat(person.toString().concat("\n")); return longString; } @Override public Iterator iterator() { return new ContactsListIterator(); } private class ContactsListIterator implements Iterator<Person> { int counter = 0; @Override public boolean hasNext() { return (counter < personList.size()); } @Override public Person next() { return personList.get(counter++); } } } 

Answer 1

First to understand the purpose, contact-list is like a contact directory, where the application holding this list, should be able add, remove and query contact easily from other service-layers, Based on this proceeding to improvement of the code,

Person.class has many type of constructors and for scalability and sustainable code, use builder pattern. Also the you can mandate certain variables as required first-name and last-name easily. Then your adding a contact to list method becomes simple.

ContactList.class has many public methods which helps in building person class, if using java 8, use optionals help in reducing these methods or the approach of method overloading is also not bad, but creates more maintainable code.

Another important suggestion is to refactor removeContacts: use only person data structure and then use optionals to remove the contacts based on the fields set, this will be neat and reduce unnecessary methods.

Further this answer summarizes refactoring neatly.

Answer 2

If your purpose is the same as stated by @sachin then my remarks are below.

Person:

• Why did you name it person instead of contact, some can think that you will manage different kinds of contacts in the future.
• Since the only usage you made of favourite is a test on it, then store it as a boolean there is no need to use a string and string comparison for that. Note also that the convention is to use is as a prefix for getters on boolean; isFavourite is a better name.
• getFullName is a computed value, you can compute it when required if you want.
• equals and hashCode are missing. Since you plan to use this class in a collection, those are two important methods. If you consider that each entry is a different one then you may keep it as is but this have many drawback.

ContactList:

• addContacts(String, String, String, String, String, boolean) can delegate to addContacts(Person) so that you have one single point to change when inserting into the list. I have used the method with all those String parameters to show the complexity of those kind of methods. If you want to follow the SOLID principles, you should better have one class (Maybe a builder like @Sachin suggets) to create a valid Person from different combination of parameters and keep your ContactList cleaner by only managing the contacts, not the creation. I would have named this method with a singular form addContact because they are used to add one contact. And maybe follow the method names used in the collections Api: add(Contact).
• getTotalContacts, again getSize() or size() are more common.
• removeContacts(String). Instead of looping trough the list to find the name then loop again in the list (via indexOf) you can just save the Person and remove it via your removeContacts(Person) method. But since you have decided to allow many contacts with the same name, you should better loop until none is found.
• displayFavouriteContacts. Again, to follow the SOLID principles, this should not be done by ContactList but by another class that use a method on ContactList to retrieve the favourites and then display them. In case you don't know, think about testsing; how could you test your method via a unit test ?
• personList. You are using an ArrayList but there may be better types of collections. And with a good implementation of equals and hashCode in Person you can benefit from them.

Finally, I am not sure that it will perform better but your code can gain by using the Java 8 streams and functional style.

You duplicate almost all the code in each getPhone..() methods, but the only variation is the predicate. By creating a method that use a predicate to filter the stream of contacts you can quickly reduce the duplication. You can also a mapping function to select the properties that you want:

public List<String> getPhoneByLastName(String lastName) { return getPhoneBy(p -> p.getLastName().equals(lastName)); } public List<String> getPhoneBy(Predicate<Person> predicate) { return collect(predicate, Person::getPhone); } private <T> List<T> collect(Predicate<Person> predicate, Function<Person, T> collector) { return personList.stream() .filter(predicate) .map(collector) .collect(Collectors.toList()); }