0

Beginner, be kind.

Whenever I add to the arraylist through the "Add Person", it overwrites the previous entry. So when I run sort, I then get two of the exact same entries which is whichever was typed in last. How do I fix this? What do I do?

Here is my class file:

import java.util.Calendar;
import java.util.Date;
import java.text.*;

public class Person implements Comparable {

    private static int totalNumber;

    public static int getTotal() {

        // Returns total number of employees
        return totalNumber;
    }

    private String fName;
    private String lName;
    private Date lastModified;
    private String address;
    private String city;
    private String state;
    private String zip;
    private String phone;

    public Person(String fName, String lName, String address, String city,
            String state, String zip, String phone) {
        this.fName = fName;
        this.lName = lName;
        this.address = address;
        this.city = city;
        this.state = state;
        this.zip = zip;
        this.phone = phone;
        Calendar calobj = Calendar.getInstance();
        this.lastModified = calobj.getTime();
    }

    public String getfName() {
        return this.fName;
    }

    public void setfName(String fName) {
        this.fName = fName;
    }

    public String getlName() {
        return this.lName;
    }

    public void setlName(String lName) {
        this.lName = lName;
    }

    public Date getLastModified() {
        return this.lastModified;
    }

    public void setLastModified(Date lastModified) {
        this.lastModified = lastModified;
    }

    public String getAddress() {
        return this.address;
    }

    public void setAddress(String address) {
        this.address = address;
    }

    public String getCity() {
        return this.city;
    }

    public void setCity(String city) {
        this.city = city;
    }

    public String getState() {
        return this.state;
    }

    public void setState(String state) {
        this.state = state;
    }

    public String getZip() {
        return this.zip;
    }

    public void setZip(String zip) {
        this.zip = zip;
    }

    public String getPhone() {
        return this.phone;
    }

    public void setPhone(String phone) {
        this.phone = phone;
    }

    @Override
    public String toString() {
        DateFormat df = new SimpleDateFormat("dd/MM/yy HH:mm:ss");
        return "\n First Name= " + fName + "\n Last Name= " + lName
                + "\n Address= " + address + "\n City= " + city + "\n State= "
                + state + "\n Zip= " + zip + "\n Phone= " + phone
                + "\n Last Modified= " + df.format(lastModified);
    }

    @Override
    public int compareTo(Object other) {
        // TODO Auto-generated method stub
        return this.lName.compareToIgnoreCase(((Person) other).lName);

    }

}

Here is my test file:

import java.util.*;

public class testAddressBook {

    public static void main(String[] args) {
        // TODO Auto-generated method stub

        ArrayList<Person> addressBook = new ArrayList<Person>();
        Person newPerson = new Person(null, null, null, null, null, null, null);

        @SuppressWarnings("resource")
        Scanner sc = new Scanner(System.in);

        boolean switcher = true;
        do {
            System.out.println("\n\tAddress Book Menu");
            System.out.println("\n\t\tEnter A to (A)dd Person ");
            System.out.println("\t\tEnter D to (D)elete Person");
            System.out.println("\t\tEnter M to (M)odify Person");
            System.out.println("\t\tEnter S to (S)earch Address Book ");
            System.out.println("\t\tEnter L to (L)ist ALL (sorted) ");
            System.out.println("\t\tEnter Q to Quit");
            System.out.print("\n\tPlease enter your choice: ");
            char choice = sc.nextLine().toUpperCase().charAt(0);

            while ((choice != 'A') && (choice != 'D') && (choice != 'M')
                    && (choice != 'S') && (choice != 'L') && (choice != 'Q')) {
                System.out
                        .println("Invalid choice!  Please select (A)dd, (D)elete, (M)odify, (S)earch, (L)ist or (Q)uit: ");
                choice = sc.nextLine().toUpperCase().charAt(0);
            }

            switch (choice) {
            case 'A':
                System.out.println("\nTo add a person, follow the prompts.");

                System.out.print("\nEnter First Name: ");
                newPerson.setfName(sc.nextLine());

                System.out.print("\nEnter Last Name: ");
                newPerson.setlName(sc.nextLine());

                System.out.print("Enter Address: ");
                newPerson.setAddress(sc.nextLine());

                System.out.print("Enter City: ");
                newPerson.setCity(sc.nextLine());

                System.out.print("Enter State: ");
                newPerson.setState(sc.nextLine());

                System.out.print("Enter Zip: ");
                newPerson.setZip(sc.nextLine());

                System.out.print("Enter Phone Number: ");
                newPerson.setPhone(sc.nextLine());

                addressBook.add(newPerson);

                System.out
                        .println("\nYou have successfully added a new person!");

                break;

            case 'D':

                break;
            case 'M':

                break;
            case 'S':
                Collections.sort(addressBook);

                for (int i = 0; i < addressBook.size(); i++) {
                    System.out.println(addressBook.get(i));

                }
                System.out.println();

                break;
            case 'L':

                break;
            case 'Q':
                switcher = false;
                System.exit(0);
                break;
            default:

            }

        } while (switcher != false);

    }
}
niyasc
  • 4,440
  • 1
  • 23
  • 50
wiredlime2015
  • 123
  • 3
  • 15

5 Answers5

1

You have to create a new instance of Person for every iteration of the loop. Otherwise, you are just updating the same Person all over again.

Move this line into the do loop:

Person newPerson = new Person(null, null, null, null, null, null, null);

Over even better, make a private method that returns a new Person and have something like

 addressBook.add(readNewPerson(sc));
 System.out.println("\nYou have successfully added a new person!");
Thilo
  • 257,207
  • 101
  • 511
  • 656
1

You create your Person only one times:

 Person newPerson = new Person(null, null, null, null, null, null, null);

And you override the properties in every loop. You have to create a new person in your loop:

   do {
        System.out.println("\n\tAddress Book Menu");
        System.out.println("\n\t\tEnter A to (A)dd Person ");
        System.out.println("\t\tEnter D to (D)elete Person");
        System.out.println("\t\tEnter M to (M)odify Person");
        System.out.println("\t\tEnter S to (S)earch Address Book ");
        System.out.println("\t\tEnter L to (L)ist ALL (sorted) ");
        System.out.println("\t\tEnter Q to Quit");
        System.out.print("\n\tPlease enter your choice: ");
        char choice = sc.nextLine().toUpperCase().charAt(0);


        while ((choice != 'A') && (choice != 'D') && (choice != 'M')  && (choice != 'S') && (choice != 'L')&& (choice != 'Q')) {
            System.out.println("Invalid choice!  Please select (A)dd, (D)elete, (M)odify, (S)earch, (L)ist or (Q)uit: ");
            choice = sc.nextLine().toUpperCase().charAt(0);
        }


        switch (choice) {
        case 'A' :      
            System.out.println("\nTo add a person, follow the prompts.");       
            Person newPerson = new Person(null, null, null, null, null, null, null);
 ....
Jens
  • 67,715
  • 15
  • 98
  • 113
1

You need to instantiate a new Person in the loop. For example declare:

Person newPerson;

And then in while loop instantiate it:

switch (choice) {
        case 'A' :      
            System.out.println("\nTo add a person, follow the prompts.");       
            newPerson = new Person(null, null, null, null, null, null, null);
 .... so on

If you also instantiate it out of the loop as well it leads to creation of one extra object.

akhil_mittal
  • 23,309
  • 7
  • 96
  • 95
0
           `Person newPerson = new Person();//define a default constructor
             . . . . 
            System.out.print("Enter Phone Number: ");
            newPerson.setPhone(sc.nextLine());

            addressBook.add(newPerson);`

You need to create a new person in side a loop and set his attributes, finally add the person to list.

amith
  • 399
  • 1
  • 2
  • 11
0

You are reusing the newPerson object over and over, just rewriting the internal fields. In Java an object (rather than primitive) variable is a named reference to an object, so every time you do addressBook.add(newPerson) you are adding the same object to the list (this is allowed because it's not a Set). You are not copying newPerson into the list, you are just making the nth position in the list point to newPerson.

The minimal change would be to move the object constructor call as Thilo suggests, but I strongly agree with his further suggestion that you should be using a dedicated method for each case. In that case, your whole problem disappears neatly due to scope of the local variable. In general you should structure your programs so that they make sense and are readable (rather than being a monolithic main procedure) and as a direct consequence all sorts of bugs just never happen.

See also:

What's the difference between passing by reference vs. passing by value?

Community
  • 1
  • 1
Béla
  • 284
  • 1
  • 2
  • 8