Address book program which saves contacts to text file

Question

package com.company;

import java.io.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

import static com.company.Person.id;


public class Main {
    private static Scanner in = new Scanner(System.in);
    private static File file = new File("Addresses.txt");
    static List<Person> people = new ArrayList<>();

    public static void main(String[] args) throws IOException {
        readPeopleFromFile();
        showMainMenu();
    }

    private static void findPerson() throws IOException {
        System.out.println("1. Find with name");
        System.out.println("2. Find with surname");

        String choice;
        do {
            choice = in.nextLine();
            switch (choice) {
                case "1":
                    findByName();
                    break;
                case "2":
                    findBySurname();
                    break;
                default:
                    System.out.print("Choose 1 or 2: ");
            }
        } while (!choice.equals("1") && !choice.equals("2"));
        System.out.println();
        showMainMenu();
    }

    private static void findBySurname() {
        System.out.print("Enter surname: ");
        String surnameToFind = in.nextLine();
        int matches = 0;
        for(Person person : people) {           
            if(person.getSurname().equals(surnameToFind)) {
                System.out.println(person);
                matches++;
            }
        }
         if(matches<=0) {
            System.out.println("There is no person with this surname");
        }
    }

    private static void findByName() {
        System.out.print("Enter name: ");
        String nameToFind = in.nextLine();
        int matches = 0;
        for(Person person : people) {       
            if(person.getName().equals(nameToFind)) {
                System.out.println(person);
                matches++;
            }
        }
        if(matches<=0) {
            System.out.println("There is no person with this name ");
        }
    }

    private static void addPerson() throws IOException {

        System.out.println("Enter name: ");
        String name = in.nextLine();
        System.out.println("Enter surname: ");
        String surname = in.nextLine();
        System.out.println("Enter phone number: ");
        String phoneNumber = in.nextLine();
        System.out.println("Enter email: ");
        String email = in.nextLine();
        System.out.println("Enter addres: ");
        String address = in.nextLine();

        Person person = new Person(name, surname, phoneNumber, email, address);
        addToFile(person);
        people.add(person);
        System.out.println("Added person number: " + id + person);
        System.out.println();
        showMainMenu();
    }

    private static void addToFile(Person person) {
            try(BufferedWriter writer = new BufferedWriter(new FileWriter(file, true))) {
                writer.write(person.getName()+"\r\n" + person.getSurname() + "\r\n" + person.getPhoneNumber() + "\r\n" + person.getEmail() +
                "\r\n" + person.getAddress() + "\r\n\r\n");
            } catch(IOException e) {
                System.out.println(e);
            }
    }

    private static boolean readPeopleFromFile() throws IOException {
        try(BufferedReader reader = new BufferedReader(new FileReader(file))) {
            String name = null;
            while((name = reader.readLine()) != null) {
                Person person = new Person(name, reader.readLine(), reader.readLine(), reader.readLine(), reader.readLine());
                people.add(person);        //adds person to the list
                reader.readLine();                                 
            }
            return true;
        }
        catch ( IOException e) {
            System.out.println(e);
        }
        return false;
    }

    private static void showMainMenu() throws IOException {
        System.out.println("1. Add person");
        System.out.println("2. Find person");
        System.out.println("3. Show all contacts");
        System.out.println("4. Close program");

        String choice;
        do {
            choice = in.nextLine();
            switch (choice) {
                case "1":
                    addPerson();
                    break;
                case "2":
                    findPerson();
                    break;
                case "3":
                    System.out.println(people);
                    System.out.println();
                    showMainMenu();
                    break;
                case "4":
                    System.exit(0);
                    break;
                default:
                    System.out.println("Enter numer from 1 to 4");
            }
        }while(!choice.equals("4"));
    }
}

And the Person class:

package com.company;

public class Person {
    static int id = Main.people.size();
    private String name;
    private String surname;
    private String phoneNumber;
    private String email;
    private String address;

    Person(String name, String surname, String phoneNumber, String email, String address) {
        this.name = name;
        this.surname = surname;
        this.phoneNumber = phoneNumber;
        this.email = email;
        this.address = address;
        id++;
    }

    String getName() {
        return name;
    }

    String getSurname() {
        return surname;
    }

    String getPhoneNumber() {
        return phoneNumber;
    }

    String getEmail() {
        return email;
    }

    String getAddress() {
        return address;
    }

    @Override
    public String toString() {
        return "\n\nName: " + getName() + "\nSurname: " + getSurname() + "\nPhone number: " + getPhoneNumber() + "\nEmail: " +
                getEmail() + "\nAddress: " + getAddress();
    }
    }


My code works as I want it to, but I would like someone to look at it and help me improve it.

Answer 1

I would look a bit more into separating the classes to perform more specific functions.

1. The Person class is mostly good however there are a few things I would change/add.

   • The variable name should be switched to firstName and its getter method should be called getFirstName()
   • You need to provide a constructor that takes an id, you don't want to give a Person a new id number if they already exist in the file (you should also be saving their id in the file).
   • You want to be careful about using a static variable as your id. In a multi-threaded environment you need to ensure that 2 objects don't end up using the same ID number. In the code below I do that by using the AtomicInteger class to provide thread safety.

This is what the Person class looks like after those changes:

public class Person {
    private static AtomicInteger nextID = new AtomicInteger(1);

    private int id;
    private String firstName;
    private String surname;
    private String phoneNumber;
    private String email;
    private String address;

    Person(int id, String firstName, String surname, String phoneNumber, String email, String address) {
        this.id = id;
        this.firstName = firstName;
        this.surname = surname;
        this.phoneNumber = phoneNumber;
        this.email = email;
        this.address = address;
    }

    Person(String firstName, String surname, String phoneNumber, String email, String address) {
        this(nextID.getAndIncrement(), firstName, surname, phoneNumber, email, address);
    }

    int getID() {
        return id;
    }

    String getFirstName() {
        return firstName;
    }

    String getSurname() {
        return surname;
    }

    String getPhoneNumber() {
        return phoneNumber;
    }

    String getEmail() {
        return email;
    }

    String getAddress() {
        return address;
    }

    @Override
    public String toString() {
        return "\n\nID: " + getID() + "\nName: " + getFirstName() + "\nSurname: " + getSurname() + "\nPhone number: " + getPhoneNumber() + "\nEmail: " +
                getEmail() + "\nAddress: " + getAddress();
    }
}

2. The file acts as your database and should have its own class. I named it DataFile. Your database as you currently have it contains 2 operations, save and load(all).

The following is the DataFile class with its operations moved into it from Main:

public class DataFile {

    private File file;

    public DataFile(String fileName) {
        this.file = new File(fileName);
    }

    public DataFile(File file) {
        this.file = file;
    }

    public void save(Person person) {
        try(BufferedWriter writer = new BufferedWriter(new FileWriter(file, true))) {
            writer.write(person.getFirstName()+"\r\n" + person.getSurname() + "\r\n" + person.getPhoneNumber() + "\r\n" + person.getEmail() +
                    "\r\n" + person.getAddress() + "\r\n\r\n");
        } catch(IOException e) {
            System.out.println(e);
        }
    }

    public List<Person> loadAll() throws IOException {
        List<Person> people = new ArrayList<Person>();

        try(BufferedReader reader = new BufferedReader(new FileReader(file))) {
            String name = null;
            while((name = reader.readLine()) != null) {
                Person person = new Person(name, reader.readLine(), reader.readLine(), reader.readLine(), reader.readLine());
                people.add(person);
                reader.readLine();
            }
        }
        catch ( IOException e) {
            System.out.println(e);
        }

        return people;
    }
}

Note that I changed the name of the methods from addToFile to save and from readPeopleFromFile to loadAll. This is because it allows us to further abstract the method in the future. This entire class allows us to create multiple files for our database, each one storing different objects. For example if we decided to create a Company class and allow the user to store the names of contacts that may be a Company (instead of a Person) we can easily do so. They simply create another instance of DataFile and pass in the name of the new file they want to use to store Companys and we can switch the name of the parameters in save() to take an object and we can technically store any object.

Loading would be a bit more difficult, but by making the method generic we can store the name of the field along with the data and use reflection to set their values.

NOTE: there are better ways to do all of this, but this is obviously a very primitive and basic database, I'm trying to show the benefits of allow abstraction rather than the specific implementation.

3. Get out of Main ASAP. As a general rule you want to use the main method to launch your program, but then you want to get out and move to a class whose job it is to run it. I created the AddressBookController class to do just that.

This is the code and below is an explanation on some of the changes I made:

public class AddressBookController {
    private enum Action {
        ADD_PERSON,
        FIND_PERSON,
        DISPLAY_ALL,
        EXIT
    }

    private enum FilterOption {
        FIRST_NAME,
        SURNAME
    }

    private DataFile dataFile;
    private Scanner in;
    private List<Person> people;

    public AddressBookController(DataFile dataFile) {
        in = new Scanner(System.in);
        this.dataFile = dataFile;
        try {
            people = dataFile.loadAll();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public AddressBookController(String fileName) {
        this(new DataFile(fileName));
    }

    public AddressBookController(File file) {
        this(new DataFile(file));
    }

    private List<Person> findPerson(String searchString, FilterOption filter) {
        switch (filter) {
            case FIRST_NAME: return people.stream().filter(person -> person.getSurname().equals(searchString)).collect(Collectors.toList());
            case SURNAME: return people.stream().filter(person -> person.getFirstName().equals(searchString)).collect(Collectors.toList());
            default:
                System.out.println("Invalid filter option");
                return new ArrayList<>();
        }
    }

    public void run() {
        while(true) {
            Action action = showMainMenuAndGetSelection();
            switch (action) {
                case ADD_PERSON:
                    Person person = getPersonInformation();
                    dataFile.save(person);
                    people.add(person);
                    break;
                case FIND_PERSON:
                    FilterOption selectedFilter = showFindPersonFilterOptionsAndGetSelection();
                    System.out.print("Enter name: ");
                    String searchString = in.nextLine();
                    List<Person> filteredPeople = findPerson(searchString, selectedFilter);
                    if (filteredPeople.size() == 0) {
                        System.out.println("No matches");
                    } else {
                        for (Person p : filteredPeople)
                            System.out.println(p);
                    }
                    break;
                case DISPLAY_ALL:
                    System.out.println(this.people);
                    System.out.println();
                    break;
                case EXIT:
                    System.out.println("Exiting Program");
                    System.exit(0);
                    break;
            }
        }
    }

    private Action showMainMenuAndGetSelection() {
        System.out.println("1. Add person");
        System.out.println("2. Find person");
        System.out.println("3. Show all contacts");
        System.out.println("4. Close program");
        String choice;
        do {
            choice = in.nextLine();
            switch (choice) {
                case "1": return Action.ADD_PERSON;
                case "2": return Action.FIND_PERSON;
                case "3": return Action.DISPLAY_ALL;
                case "4": return Action.EXIT;
                default: System.out.println("Enter a number from 1 to 4");
            }
        } while (!choice.equals("4"));
        return null; //should never reach here
    }

    private Person getPersonInformation() {
        System.out.println("Enter first name: ");
        String firstName = in.nextLine();
        System.out.println("Enter surname: ");
        String surname = in.nextLine();
        System.out.println("Enter phone number: ");
        String phoneNumber = in.nextLine();
        System.out.println("Enter email: ");
        String email = in.nextLine();
        System.out.println("Enter addres: ");
        String address = in.nextLine();
        return new Person(firstName, surname, phoneNumber, email, address);
    }

    private FilterOption showFindPersonFilterOptionsAndGetSelection() {
        System.out.println("1. Find with name");
        System.out.println("2. Find with surname");
        System.out.println();
        String choice;
        do {
            choice = in.nextLine();
            switch (choice) {
                case "1":  return FilterOption.FIRST_NAME;
                case "2": return FilterOption.SURNAME;
                default: System.out.print("Choose 1 or 2");
            }
        } while (!choice.equals("1") && !choice.equals("2"));
        return null; //should never reach here
    }
}

The main method in this class to notice is the run() method. It's what starts the program (and it is the only public method outside of the constructors).

It uses an infinite loop to constantly show the main menu (this way I never need to call it from inside another method). However instead of using ints to control the flow of the program, we should use enums. This makes the flow of the program easier to read.

Another change is that instead of the methods displaying the menu's and then deciding where to act from there, they pass that information back to the run method which decides how to proceed. This keeps us from getting to deep into a method stack and only the run method is controlling the entire program.

I also simplified the findPerson function. It takes the string to search for and the filter option (these are passed in by the run function). It uses the Java 8 streams. This way we don't need to create a brand new method for each filter we want to add. (we just change which field it is comparing it too.

4. The final piece of the puzzle is putting in our main method.

   public class Main {
        public static void main(String[] args) {
            AddressBookController controller = new AddressBookController("data_file.txt");
            controller.run();
        }
    }
   

As you can see, very simple. We create the object and run it. The main method itself does very little.

EDIT: One thing I didn't do was change the save and load function to read ID's. However this should be done since we don't to end up giving the same people different ID's (this likely isn't an issue here, since it all in a single file, but it could be an issue if you want to use some sort of foreign key structure where another file references the ID of a Person)

Answer 2

Advice 1

private static boolean readPeopleFromFile() { ... }

In case you or someone else is to maintain your code, it would be nicer if readPeopleFromFile would take a File object as input and return a List<Person>:

private static List<Person> readPeopleListFromFile(File file) { ... }

Advice 2

private static void addToFile(Person person) { ... }

would be better as

private static void addPersonToFile(Person person, File file) { ... }

Advice 3

private static void showMainMenu() { ... }

I would do exactly what was implied by the name of the method: show the menu; plus, I would added a distinct method for asking the user for input.

Advice 4

choice = in.nextLine();

if your write

choice = in.nextLine().trim(); // trim() removes all whitespace from the beginning and end of the string returned by in.nextLine().

you will make your input handling a little bit more robust.

Advice 5

static int id = Main.people.size();

This is suffient in your program, but in software industry it would be more reasonable to manage ID computation in the business logic (Main.java in your case).

Hope that helps.

Answer 3

One more thing in addition to the other answers: nowadays I'd leave the construction of buffered readers and writers to the appropriate utility class, ie. instead of

BufferedReader reader = new BufferedReader(new FileReader(file))

use

BufferedReader reader = Files.newBufferedReader(file.toPath())

(or even better: make file a java.nio.files.Path in the first place)

The same goes for the buffered writer and Files.newBufferedWriter()