Command line Contact Management

Question

Can you please review the following code and give any suggestions for improvment?

Class ContactList.java

import java.io.*;
import java.util.*;

public class ContactList {

public static void main(String args[]) throws IOException {
    Contact contact;
    contact = new Contact();
    int action = 0;

    ArrayList<Contact> contacts = new ArrayList<Contact>();
    while (action != 6) {

        System.out.println("\nWelcome to Contact List DB. "
                + "What would you like to do? \n");

        System.out.println("1. Enter a new person" + "\n"
                + "2. Print the contact list" + "\n"
                + "3. Retrieve a person's information by last name" + "\n"
                + "4. Retrieve a person's information by email address" + "\n"
                + "5. Retrieve all people who live in a given zip code" + "\n" 
                + "6. Exit");
        Scanner reader = new Scanner(System.in);
        reader.useDelimiter("\n");
        action = reader.nextInt();

        if (action <= 0 || action > 6) {
            System.out.println("Invalid selection. ");

        }

        switch (action) {

        case 1: {

            System.out.println("\nEnter Contact Last Name:");
            String lastname = reader.next();
            if (lastname == null) {
                System.out.println("\nInvalid entry. ");
                break;
            } 

            else {
                contact.setLastName(lastname.toLowerCase());
            }
            System.out.println("Enter Contact First Name: ");
            String firstname = reader.next();
            contact.setFirstName(firstname.toLowerCase());
            System.out.println("Enter Contact Street Address: ");
            String address = reader.next();
            contact.setHouseAddress(address.toLowerCase());
            System.out.println("Enter Contact City: ");
            String city = reader.next();
            contact.setCity(city.toLowerCase());
            System.out.println("Enter Contact Zip Code: ");
            String zip = reader.next();
            contact.setZip(zip.toLowerCase());
            System.out.println("Enter Contact Email: ");
            String email = reader.next();
            contact.setEmail(email.toLowerCase());
            System.out.println("Enter Contact Phone Number: ");
            String phone = reader.next();
            contact.setPhone(phone.toLowerCase());
            System.out.println("Enter Contact Notes: ");
            String notes = reader.next();
            contact.setNotes(notes.toLowerCase());

            contacts.add(contact);

            try {

                Contact c = contact;

                File file = new File("contactlist.csv");

                // If file doesn't exists, then create it.
                if (!file.exists()) {
                    file.createNewFile();
                }

                try (PrintWriter output = new PrintWriter(new FileWriter(
                        "contactlist.csv", true))) {
                    output.printf("%s\r\n", c);
                } catch (Exception e) {
                }

                System.out.println("Your contact has been saved.");
            }

            catch (IOException e) {
                e.printStackTrace();        
    }
        }

        break;


        case 2: {

            int counter = 0;
            String line = null;

            // Location of file to read
            File file = new File("contactlist.csv");

            // Sort contacts and print to console
            try {

                Scanner scanner = new Scanner(file);

                // Before printing, add each line to a sorted set. by Seth
                // Copeland
                Set<String> lines = new TreeSet<>();
                while (scanner.hasNextLine()) {
                    line = scanner.nextLine();
                    lines.add(line);
                    counter++;

                }

                // Print sorted contacts to console.
                for (String fileLine : lines) {
                    String outlook = fileLine.substring(0, 1).toUpperCase()
                            + fileLine.substring(1);
                    System.out.println(outlook);

                }


                scanner.close();

            } catch (FileNotFoundException e) {

            }
            System.out.println("\n" + counter + " contacts in records.");

        }

        break;


        case 3:

            try {
                System.out.println("\nEnter the last"
                        + "name to search for: ");
                String searchterm = reader.next();

                // Open the file as a buffered reader
                BufferedReader bf = new BufferedReader(new FileReader(
                        "contactlist.csv"));

                // Start a line count and declare a string to hold our
                // current line.
                int linecount = 0;
                String line;

                // Let the user know what we are searching for
                System.out.println("Searching for " + searchterm
                        + " in file...");
                // Loop through each line, putting the line into our line
                // variable.
                boolean noMatches = true;
                while ((line = bf.readLine()) != null) {
                    // Increment the count and find the index of the word.
                    linecount++;
                    int indexfound = line.indexOf(searchterm.toLowerCase());

                    // If greater than -1, means we found a match.
                    if (indexfound > -1) {
                        System.out.println("\nContact was FOUND\n"
                                + "\nContact " + linecount + ": " + line);
                        noMatches = false;
                    }

                }

                // Close the file after done searching
                bf.close();
                if (noMatches) {
                    System.out.println("\nNO MATCH FOUND.\n");
                }
            }

            catch (IOException e) {
                System.out.println("IO Error Occurred: " + e.toString());
            }

            break;


        case 4:

            try {
                System.out.println("\nEnter the email "
                        + "address to search for: ");
                String searchterm = reader.next();

                // Open the file as a buffered reader
                BufferedReader bf = new BufferedReader(new FileReader(
                        "contactlist.csv"));

                // Start a line count and declare a string to hold our
                // current line.
                int linecount = 0;
                String line;

                // Let the user know what we are searching for
                System.out.println("\nSearching for " + searchterm
                        + " in file...");

                // Loop through each line, put the line into our line
                // variable.
                boolean noMatches = true;
                while ((line = bf.readLine()) != null) {

                    // Increment the count and find the index of the word
                    linecount++;
                    int indexfound = line.indexOf(searchterm.toLowerCase());

                    // If greater than -1, means we found a match
                    if (indexfound > -1) {
                        System.out.println("\nContact was FOUND\n"
                                + "\nContact " + linecount + ": " + line);
                        noMatches = false;
                    }

                }
                // Close the file after done searching
                bf.close();
                if (noMatches) {
                    System.out.println("\nNO MATCH FOUND.\n");
                }

            }

            catch (IOException e) {
                System.out.println("IO Error Occurred: " + e.toString());
            }

            break;


        case 5:

            try {
                System.out.println("\nEnter the Zipcode to search for: ");
                String searchterm = reader.next();

                // Open the file as a buffered reader
                BufferedReader bf = new BufferedReader(new FileReader(
                        "contactlist.csv"));

                // Start a line count and declare a string to hold our
                // current line.
                int linecount = 0;
                String line;

                // Let the user know what we are searching for
                System.out.println("\nSearching for " + searchterm
                        + " in file...");

                // Loop through each line, stashing the line into our line
                // variable.
                boolean noMatches = true;
                while ((line = bf.readLine()) != null) {

                    // Increment the count and find the index of the word.
                    linecount++;
                    int indexfound = line.indexOf(searchterm.toLowerCase());

                    // If greater than -1, means we found a match.
                    if (indexfound > -1) {
                        System.out.println("\nContact was FOUND\n"
                                + "\nContact " + linecount + ": " + line);
                        noMatches = false;
                    }
                }
                // Close the file after done searching
                bf.close();
                if (noMatches) {
                    System.out.println("\nNO MATCH FOUND.\n");
                }
            }

            catch (IOException e) {
                System.out.println("IO Error Occurred: " + e.toString());
            }

            break;
        }
    }
}
}

And here's Contact.java

    import java.io.File;
    import java.io.FileWriter;
    import java.io.IOException;
    import java.io.PrintWriter;

public class Contact {
private String lastname, firstname, address, city, zip, email, phone,
        notes;

public Contact(String lastnamename, String firstname, String address,
        String city, String zip, String email, String phone, String notes,
        String lastname) {
    this.lastname = lastname;
    this.firstname = firstname;
    this.address = address;
    this.city = city;
    this.zip = zip;
    this.email = email;
    this.phone = phone;
    this.notes = notes;
}

public Contact() {

}

// overrides the default Object method
public String toString() {
    return lastname + ", " + firstname + ", " + address + ", " + city
            + ", " + zip + ", " + email + ", " + phone + ", " + notes;
}

/*
 * Sets the value for lastname to "s".
 */
public void setLastName(String s) {
    lastname = s;
}

/*
 * Returns the value of lastname.
 */
public String getLastName() {
    return lastname;
}

/*
 * Sets the value for firstname to "a".
 */
public void setFirstName(String a) {
    firstname = a;
}

/*
 * Returns the value of firstname.
 */
public String getFirstName() {
    return firstname;
}

/*
 * Sets the value for address to "b".
 */
public void setHouseAddress(String b) {
    address = b;
}

/*
 * Returns the value of address.
 */
public String getHouseAdress() {
    return address;
}

/*
 * Sets the value for city to "c".
 */
public void setCity(String c) {
    city = c;
}

/*
 * Returns the value of city.
 */
public String getCity() {
    return city;
}

/*
 * Sets the value for zip to "d".
 */
public void setZip(String d) {
    zip = d;
}

/*
 * Returns the value of zip.
 */
public String getZip() {
    return zip;
}

/*
 * Sets the value for phone to "e".
 */
public void setPhone(String e) {
    phone = e;
}

/*
 * Returns the value of phone.
 */
public String getPhone() {
    return phone;
}

/*
 * Sets the value for email to "f".
 */
public void setEmail(String f) {
    email = f;
}

/*
 * Returns the value of email.
 */
public String getEmail() {
    return email;
}

/*
 * Sets the value for notes to "g".
 */
public void setNotes(String g) {
    notes = g;
}

/*
 * Returns the value of notes.
 */
public String getNotes() {
    return notes;
}

public void read() {

}

static void write() {
    // Writes contact info to file. -Damani
            // ----------------------------------------------------------
            try {
                Contact contact;
                contact = new Contact();
                Contact c = contact;

                File file = new File("contactlist.csv");

                // If file doesn't exists, then create it.
                if (!file.exists()) {
                    file.createNewFile();
                }

                try (PrintWriter output = new PrintWriter(new FileWriter(
                        "contactlist.csv", true))) {
                    output.printf("%s\r\n", c);
                } catch (Exception e) {
                }

                System.out.println("Your contact has been saved.");
            }

            catch (IOException e) {
                e.printStackTrace();        
    }
}
}

A first glance : in this case I replace the switch by a Observable/Observer patten, loosely coupled and evolutive — cl-r
– cl-r, Commented Mar 12, 2013 at 15:30
Please give your question a meaningful title and also provide with an explanation of what your code, how it works and why it does work the way it does. — Bobby
– Bobby, Commented Mar 12, 2013 at 16:02

Jeff Vanzella · Accepted Answer · 2013-03-12 18:34:21Z

The function you wrote is too long, and handles too many concerns. I would separate it out into either other functions, classes, or preferably, a combination of both.

Comments are overused. Comments should be used to enhance the code, i.e. add an explanation why something was done a certain way. They should not describe what is being done. If you have to use comments, the section of code is probably better off in a function with a descriptive name.

You don't need to instantiate a new Contact early in your main function. That should be done just before it is needed (like where you create a new contact);

The actions should be put into an enum:

public enum MainMenuAction {
   AddContact,
   PrintContactList,
   FindPersonByLastName,
   FindPersonByEmail,
   SearchByZipCode,
   Quit,
   UnknownCommand
}

I would move the file operations into their own class, using something like a repository pattern. In fact, the ArrayList<Contact> should be put in this class too.

Create a class to handle the main loop, something like

public class ApplicationRunner {

   private Scanner reader;

   public ApplicationRunner(Scanner reader) {

       this.reader = reader;
   }

   public void run() {

      MainMenuAction action;
      while (action = readAction() != MainMenuAction.Quit) {

         switch(action) {

             case MainMenuAction.AddContact:
                 addAContact();
                 break;
             case MainMenuAction.PrintContactList:
                 printContactList();
                 break;
             // Code the rest of the actions here
             case MainMenuAction.UnknownCommand:
                 System.out.println("Invalid selection. ");
                 break;
         }
      }
   }

   private MainMenuAction readAction() {

       System.out.println("1. Enter a new person" + "\n"
            + "2. Print the contact list" + "\n"
            + "3. Retrieve a person's information by last name" + "\n"
            + "4. Retrieve a person's information by email address" + "\n"
            + "5. Retrieve all people who live in a given zip code" + "\n" 
            + "6. Exit");

        int action = reader.nextInt();

        // This might be able to be done more efficiently, I'm not that versed in
        // java syntax.
        switch (action) {
           case 1:
              return MainMenuAction.AddContact;
           // Add rest here
           default:
              return MainMenuAction.UnknownCommand;
   }
}

You have created a Constructor for Contact that takes all of the required values, why not use it. Read all the values into variables (or a Map), then at the end, create the contact with what's there. Data validation should be done in your Contact class, this will ensure invalid data will never appear in it.

The reading of the attributes could be encapsulated into a class:

public class ConsoleContactReader {

   private Scanner reader;
   private Map<string, string> enteredValues;

   public ConsoleReader(Scanner reader) {

       this.reader = reader;
       enteredValues = new Map<string, string>();
   }

   public void readConsole(string prompt, string fieldName) {

       System.out.printl(prompt);
       enteredValues.put(fieldName, reader.next());
   }

   public Contact createContactFromMap() {

      return new Contact(
          this.enteredValues.get("lastName"),
          this.enteredValues.get("firstName"),
          // etc...
          );
  }

In the addAContact function:

 public void addAContact()
 {
     ConsoleContactReader contactReader = new ConsoleContactReader(this.reader);

     contactReader.readConsole("Enter Contact Last Name: ", "lastName");
     contactReader.readConsole("Enter Contact First Name: ", "firstName");
     // continue with logic

     Contact newContact = contactReader.createContactFromMap();
     this.repository.Add(newContact);
 }

Add a repository that will deal with the reading/writing and searching of contacts

public class ContactRepository {

   private ArrayList<Contact> contacts;
   private ContactFileOperation fileOperations;

   public ContactRepository(ContactFileOperation fileOperations) {

       this.fileOperations = fileOperations;

       this.contacts = new ArrayList<Contact>();
   }

   public void addAContact(Contact contact)
   {
       contacts.add(contact);
       fileOperations.save(contact);
   }

   // Implement other operations here

}

Add a ContactFileOperationsClass which contains all your file read/write logic

public class ContactFileOperation {

   private string path;

   public ContactFileOperation(string path) {

      this.path = path;

      createFileIfRequired();
   }

   private File createFileIfRequired() {

      File file = new File(this.path);

      if (!file.exists()) {
         file.createNewFile();
      }
   }

   private PrintWriter createPrintWriter() {

       return new PrintWriter(new FileWriter(this.Path, true))
   }

   public save(Contact contact) {

       try (PrintWriter output createPrintWriter()) {
                output.printf("%s\r\n", c);
       } catch (Exception e) {
           // NEED TO DO SOMETHING WITH THIS EXCEPTION OR IT WILL SLIP AWAY UNNOTICED
       }
   }
}

Your main function could now be something like:

public class ContactList {

   public static void main(String args[]) throws IOException {

      Scanner reader = new Scanner(System.in);
      reader.useDelimiter("\n");

      ApplicationRunner runner = new ApplicationRunner(reader);

      runner.run;
}

This isn't perfect, but there should be enough here to start with. You should be able to take what I've suggested and apply it to the rest of your code.

You'll have to excuse me if I have some syntax incorrect, I don't have a compiler, and I don't work with Java that often currently.

Thanks again Jeff! This will help me improve my code and make it more efficient! — user2149907
– user2149907, Commented Mar 12, 2013 at 21:26

WarrenFaith · Accepted Answer · 2013-03-12 16:08:55Z

2

You can also reduce this (occurred multiple times)

String firstname = reader.next();
contact.setFirstName(firstname.toLowerCase());

with this

contact.setFirstName(reader.next().toLowerCase());

answered Mar 12, 2013 at 16:08

WarrenFaith

1732 silver badges9 bronze badges

\$\begingroup\$ Your code was very helpful as well! \$\endgroup\$

user2149907
– user2149907

2013-03-14 02:54:23 +00:00
Commented Mar 14, 2013 at 2:54

Add a comment |

FrustratedWithFormsDesigner · Accepted Answer · 2013-03-12 15:37:40Z

1

The first thing I would suggest is to replace your magic numbers with meaningful constants:

private static final int NEW_PERSON_ACTION = 1;
private static final int PRINT_PERSON_ACTION = 2;
private static final int SEARCH_LAST_NAME_ACTION = 3;
//etc...

answered Mar 12, 2013 at 15:37

FrustratedWithFormsDesigner

4843 silver badges11 bronze badges

Add a comment |

user23094 · Accepted Answer · 2013-03-13 14:06:44Z

ContactList should be what it says it is - something meaningfully related to a list of contacts.

It should probably have these methods to:
-addContact()
-removeContact()
-numContacts()
-containsLastName()
-replaceContact(old, new)
-getContact(lastName)
-getAllContacts()

Contact has that static method write but it's actually useless (it doesn't do anything- it just creates an empty contact and prints it...) and a bad idea to put it there anyway. You don't want to put File I/O in random spots in your code. Also you don't need comments on your get/set methods, they don't do anything more then their name says. Use comments to illustrate the broad scope of your code, or to explain how a confusing part works.

The main method could be better suited in some method such as ContactHarness and basically put all the parts of your code that aren't related to ContactList or Contact in there. That would be your application loop and your file i/o.

Enums are an improvement over 'private static final int' but in this case, I'd probably just use final ints because it's really simple. These constants would be declared in your ContactHarness/whatever had your loop. You want to avoid 'magic numbers', or numbers that have special meanings in your program, and replace them with appropriately named variables.

I'd replace lines like
line = scanner.nextLine()
lines.add(line)
with lines.add(scanner.nextLine())

But beyond that I can't understand what your program is trying to do. I mean I kind of do, but I don't know why it does it that way. Why are you constantly reading to/from a file? You declare a list of in a class called ContactList which made me assume you want to use a list of contacts somewhere but you never use the list of contacts. There are reasons not to read the entire contact list into memory immediately (just like there are reasons to use databases as well), but for a beginner project I don't see why you wouldn't read your .csv file in at the start of the program, modify the List you have while the program is running, and finally have an option to export the current list as .csv or something like that.

For instance, your PRINT_LIST option doesn't even involve contacts, all it does is print lines from a file in alphabetical order.

The way you have it now, I don't even know why you have a Contact class - you only use it once. You basically just modify a .csv file a bunch of times. There's no behavior or state associated with your Contact class really. There are a bunch of style things I'd change up in your main class but I can't get over that you never use Contact/ContactList really has nothing to with a List of Contacts. Work on those general ideas first.

I'm constantly reading to and from a file because I wanted the user to always be able to I guess access the database? All the methods require to read from file correct? To search a contact do I have to read from the existing contacts file? To print existing contacts do I have to read from file? Unless their is another way? Also the print list, I wanted it to print all "existing" contacts in alphabetical order by last name. I just wanted to create and modify a contact list. Is that not the purpose of a contact list? create and modify records? I'm no sure what you are trying to say. — user2149907
– user2149907, Commented Mar 13, 2013 at 14:59

Stack Exchange Network

Command line Contact Management

4 Answers 4

You must log in to answer this question.

Hot Network Questions

Command line Contact Management

4 Answers 4

You must log in to answer this question.

Related

Hot Network Questions