Contacts list manager

Question

I just learned about arraylists and I got a mission to build Mobile phone application with contact list when you can add \ remove \ modify \ search for contacts.

If you think I should add something to the code feel free to tell me :)

MobilePhone class

public class MobilePhone{
 private String phoneNumber;
 public ArrayList<Contact> myContacts = new ArrayList<Contact>();
 public MobilePhone(String phoneNumber) {
 this.phoneNumber = phoneNumber;
 }
 public void printContactList() {
 if (!myContacts.isEmpty()) {
 for (int i = 0; i < myContacts.size(); i++) {
 System.out.println(i + 1 + ". " + "Name: " + myContacts.get(i).getName() + " || Phone number: " + myContacts.get(i).getPhoneNumber());
 }
 } else {
 System.out.println("Your contact list is empty!");
 }
 }
 public void addContact(String name, String phoneNumber) {
 if (searchContactByPhoneNumber(phoneNumber) == -1) {
 Contact contact = new Contact(name, phoneNumber);
 myContacts.add(contact);
 System.out.println("Contact " + name + " with phone number " + phoneNumber + " just added!");
 } else {
 System.out.println("This contact is already on your list.");
 }
 }
 public void removeContact(String phoneNumber) {
 int index = searchContactByPhoneNumber(phoneNumber);
 if (index >= 0) {
 System.out.println("You have removed " + myContacts.get(index).getName());
 myContacts.remove(index);
 }
 }
 public int searchContactByPhoneNumber(String phoneNumber) {
 for (int i = 0; i < myContacts.size(); i++) {
 if (phoneNumber.equals(myContacts.get(i).getPhoneNumber())) {
 System.out.println(myContacts.get(i).getName() + " Found!");
 return i;
 }
 }
 return -1;
 }
 public int searchContactByName(String name){
 for(int i =0; i<myContacts.size(); i++){
 if(name.equals(myContacts.get(i).getName())) {
 return i;
 }
 }
 return -1;
 }
 public void changeContact(String oldName, String newName) {
 int index = searchContactByName(oldName);
 if(index >=0){
 Contact updatedContact = new Contact(newName,myContacts.get(index).getPhoneNumber());
 myContacts.set(index,updatedContact);
 System.out.println("You have changed contact " +oldName + " to " + newName + "\n" +
 "Phone number: " +myContacts.get(index).getPhoneNumber());
 } else {
 System.out.println("No contact named " + oldName + " on your contact list");
 }
 }
}

Contact class

public class Contact {
 private String name;
 private String phoneNumber;
 public Contact(String name, String phoneNumber) {
 this.name = name;
 this.phoneNumber = phoneNumber;
 }
 public String getName() {
 return this.name;
 }
 public String getPhoneNumber(){
 return this.phoneNumber;
 }
}

PrintService class

public class PrintService {
 public static void printMenu() {
 System.out.println("Press:" + "\n" +
 "\r" + "1. Show contact list" + "\n" +
 "\r" + "2. Add an contact" + "\n" +
 "\r" + "3. Remove an contact" +"\n" +
 "\r" + "4. Search for an contact" + "\n" +
 "\r" + "5. Change info about some contact" +"\n"+
 "\r" + "6. Exit.");
 }
}

Main class

public class Main {
 private static final Scanner sc = new Scanner(System.in);
 private static final MobilePhone mobilePhone = new MobilePhone("123456789");
 public static void main(String[] args){
 boolean exitRequested = false;
 while(!exitRequested) {
 PrintService.printMenu();
 int options = Integer.parseInt(sc.nextLine());
 switch (options) {
 case 1:
 mobilePhone.printContactList();
 break;
 case 2:
 addContact();
 break;
 case 3:
 removeContact();
 break;
 case 4:
 searchContact();
 break;
 case 5:
 changeContact();
 break;
 case 6:
 exitRequested = true;
 break;
 }
 }
 }
 private static void addContact() {
 System.out.println("Name?");
 String name = sc.nextLine();
 System.out.println("Phone number:");
 String phoneNumber = sc.nextLine();
 if (phoneNumber.length() != 10) {
 System.out.println("Wrong input!");
 } else {
 mobilePhone.addContact(name, phoneNumber);
 }
 }
 private static void removeContact(){
 System.out.println("Which contact would you like to remove?" +"\n" +
 "Please type phone number.");
 String phoneNumber = sc.nextLine();
 mobilePhone.removeContact(phoneNumber);
 }
 private static void searchContact(){
 System.out.println("Please enter phone number ");
 String phoneNumber = sc.nextLine();
 if(mobilePhone.searchContactByPhoneNumber(phoneNumber) == -1) {
 System.out.println("No contact found with phone number " + phoneNumber);
 } else {
 mobilePhone.searchContactByPhoneNumber(phoneNumber);
 }
 }
 private static void changeContact(){
 System.out.println("Which contact would you like to modify?");
 String currentName = sc.nextLine();
 System.out.println("Enter your modify");
 String updatedName = sc.nextLine();
 mobilePhone.changeContact(currentName,updatedName);
 }
}

Thank you very much!

• chptr-one: Just one small note. Do not use the implementation instead of an interface. Instead of public ArrayList<Contact> myContacts = new ArrayList<Contact>(); use List<Contact> myContact = new ArrayList<>(). Also you do not need to repeat the generic type twice, use a diamond operator

Answer 1

What I like

You follow the Java Naming conventions and the names you choose for your identifiers are pretty good.

What I don't like

Unnecessary mutability

The member variable phoneNumber in your class MobilePhone does not change during the objects life time, therefore it should be declared final. The same applies to name and phoneNumber in your class Contact.

Inappropriate visibility

The member variable myContacts in your class MobilePhone is declares public. That means that its content can be changed from anywhere outside the class MobilePhone. This violates the encapsulation/information hiding principle, one of the most important concepts of object oriented programming.

Using indexes instead of objects

Your search* methods return indexes instead of the real objects. Therefore the caller of that methods must access the list again in case it wants to do something with the data of that particular contact (which is almost always the case...). So you better return the contact object itself.

Inline signalling of errors

In case your search* method finds nothing you return a special value. In Java we have the concept of Exceptions to handle this kind of Problem.

Yes, there is a rule, that you should not miss use Exceptions as control flow. But they are exactly for this particular purpose, avoiding this in band signalling of a special case when returning a processing result.

In your small project Exceptions will basically replace the if/else statements with try/catch blocks which does not look like a benefit at all. But in larger projects it is very likely that the error case can not be handled by the direct calling method but by some other method way up in the call stack. Then only the code able to deal with that problem needs to know that it may occur. Without the use of exceptions any method way down in the call stack needs this if statements.

Another way to go around this is the use of Optional as the return value. But that is just another special value in my opinion.

Naming

As I initially wrote your naming is pretty good, with one minor exception. In your main method the local variable options will always contain a single user choice. Therefore it should not have the plural s.
Having written that, the name option might not be that good at all and should rather be selectedOption or just choice...

---

Answer to comment

I've tried before to make all methods (Contact contact) but fo real I got stuck hard. – עמית שוקרון

That should be quite easy.

public class MissingContact extends RuntimeException{
 public MissingContact(String message){
 super(message);
 }
}
public Contact searchContactByPhoneNumber(String phoneNumber) {
 for (contact: myContacts) {
 if (phoneNumber.equals(contact.getPhoneNumber())) {
 System.out.println(contact.getName() + " Found!");
 return contact;
 }
 }
 throw new MissingContact("No contact with number "+phoneNumber);
} // search by name similar

To avoid the Exception handling you should apply the check, then act pattern by adding appropriate check methods:

public boolean hasContactWithPhoneNumber(String phoneNumber) {
 for (contact: myContacts) {
 if (phoneNumber.equals(contact.getPhoneNumber())) {
 System.out.println(contact.getName() + " Found!");
 return true;
 }
 }
 return false;
} // searchByName similar

• עמית שוקרון: Hi ! thank you for your answer! I've tried before to make all methods (Contact contact) but fo real I got stuck hard. I coudlnt do it alone.. hope someone will guide me to it ;)
• Timothy Truckle: @עמיתשוקרון: see the update, does that help?
• עמית שוקרון: Thank you very much sir, it helped me a lot !
• עמית שוקרון: Hi again! hope you'll see it, should I change only the search method or I need to change every single method to get Contact contact parameter?
• Timothy Truckle: @עמיתשוקרון Your API should be consistent so any method that currently returns an index should return a Contact object instead.

Answer 2

I think Timothy Truckle has covered a lot of good ground here.

I'd disagree with the idea of throwing an unchecked exception when entries can't be found, however.

This exercise has very similar characteristics to the java.util.Map interface (in fact, if the OP wasn't specifically focussing on learning ArrayLists, a Map would be a better structure with which to implement a contact list).

Following the example of Map, if entries can't be found then simply return null.

I'd be inclined to make the Contact class a nested class in MobilePhone, as I don't believe it's needed elsewhere, and I think it needs a good toString() method.

I'm worried by the changeContact() operation - the MobilePhone.addContact() method only enforces unique phone numbers but the changeContact searches by name, so I don't think you're guaranteed to change the name for the contact you might have intended.

I haven't tried to address that problem in the code below (which is untested, so regard it as a sketch of how I'd do this, if forced to use ArrayList for the purpose).

import java.util.ArrayList;
public class MobilePhone {
 private static class Contact {
 private String name;
 private String phoneNumber;
 public Contact(String name, String phoneNumber) {
 this.name = name;
 this.phoneNumber = phoneNumber;
 }
 @Override
 public String toString() {
 return String.format("Name: %s , || Phone number: %s", name, phoneNumber);
 }
 }
 @SuppressWarnings("unused")
 private final String myPhoneNumber; // Different name to the contact phone number, for clarity
 private final List<Contact> myContacts = new ArrayList<Contact>(); // use Interface as the field type...
 public MobilePhone(String phoneNumber) {
 myPhoneNumber = phoneNumber;
 }
 public void printContactList() {
 if (!myContacts.isEmpty()) {
 for (Contact contact : myContacts) {
 System.out.println(contact);
 }
 }
 else {
 System.err.println("Your contact list is empty!");
 }
 }
 public void addContact(String name, String phoneNumber) {
 Contact existingContact = searchContactByPhoneNumber(phoneNumber);
 if (existingContact == null) {
 Contact contact = new Contact(name, phoneNumber);
 myContacts.add(contact);
 System.out.format("Contact %s added%n", contact);
 }
 else {
 System.err.format("This phone number is already on your list - %s%n", existingContact);
 }
 }
 public void removeContact(String phoneNumber) {
 Contact existingContact = searchContactByPhoneNumber(phoneNumber);
 if (existingContact != null) {
 myContacts.remove(existingContact); // we're ignoring the boolean result in this case
 System.out.format("You have removed contact - %s%n" + existingContact);
 }
 }
 public Contact searchContactByPhoneNumber(String phoneNumber) {
 for (Contact contact : myContacts) {
 if (phoneNumber.equals(contact.phoneNumber)) {
 return contact;
 }
 }
 return null;
 }
 public Contact searchContactByName(String name) {
 for (Contact contact : myContacts) {
 if (name.equals(contact.name)) {
 return contact;
 }
 }
 return null;
 }
 public void changeContact(String oldName, String newName) {
 Contact existingContact = searchContactByName(oldName);
 if (existingContact != null) {
 existingContact.name = newName;
 System.out.format("You have changed contact %s to %s%nPhone number: %s%n", oldName, newName, existingContact.phoneNumber);
 }
 else {
 System.err.format("No contact named %s on your contact list%n", oldName);
 }
 }
}

• Mark Bluemel: I forgot to ask - is there any chance of concurrent use of the MobilePhone class? If so, synchronization needs to be considered...