Ygthkb

For self-study homework I made a Java program that takes user input, creates people as objects, and says "Happy Birthday" to each person.

I am just wondering what mistakes I made here or how it could have been better.

PERSON CLASS

public class PeopleObjects{

    // For practice, make the people objects rather than just a list of names.



    private String name;

    private String birthDay;



    public void setName(String name){

        this.name = name;

    }



    public String getName(){

        return name;

    }



    public void setBirthDay(String myBirth){

        birthDay = myBirth;

    }



    public String getBirthDay(){

        return birthDay;

    }



}

MAIN

import java.util.Scanner;

import java.util.ArrayList;



public class PeopleObjectsTestDrive{

    public static void main(String [] args){



        ArrayList<PeopleObjects> listOfPeopleObjects = new ArrayList<PeopleObjects>();

        Scanner input = new Scanner(System.in);



        int userChoice = 0;



        System.out.println("Enter any number other than 1 to start program."+"n");



            while ((userChoice = input.nextInt()) != 1){



                PeopleObjects people = new PeopleObjects();



                System.out.println("n###What is the name of the person?###n");

                input.nextLine();//for some reason we have to use this twice to avoid bug

                people.setName(input.nextLine());



                System.out.println("n###What is their birthday?###n");

                people.setBirthDay(input.nextLine());

                listOfPeopleObjects.add(people);



                System.out.println("nDo you want to quit and see list? Enter 1 for yes and 0 for non");

            }



            System.out.println("nnnn");



            for(int i=0; i < listOfPeopleObjects.size(); i++){



                if(listOfPeopleObjects.get(i).getName().equals("Joe")){

                    System.out.println("Happy Birthday Joe! You are special! Your birthday is on "+listOfPeopleObjects.get(i).getBirthDay()+" n");

                }else if(i%2 == 0){

                    System.out.println("Happy Birthday to "+listOfPeopleObjects.get(i).getName()+".n");

                }else{

                    System.out.println("Happy Birthday to "+listOfPeopleObjects.get(i).getName()+" too!!!n");



                }//end if else 

            }//end for

    }

}

Why nextLine after nextInt?

            while ((userChoice = input.nextInt()) != 1){



                PeopleObjects people = new PeopleObjects();



                System.out.println("n###What is the name of the person?###n");

                input.nextLine();//for some reason we have to use this twice to avoid bug

While an explanatory comment is helpful, you may want to go ahead and search out the explanation: essentially, the nextInt only reads as far as the end of the number; it does not read the succeeding end-of-line. So if you want to read the next line, you have to first read that end-of-line so that you can start reading after it. Once you do that, you might move that line higher, right after the nextInt.

I prefer comments to be on separate lines, although others disagree.

Favor interfaces over implementations

        ArrayList<PeopleObjects> listOfPeopleObjects = new ArrayList<PeopleObjects>();

When defining a type, the general preference is to use the interface rather than the implementation. So

        List<Person> people = new ArrayList<>();

This encourages you to code to the interface, only using methods that are available in the interface. That in turn makes it easier to switch implementations.

In the more recent versions of Java, you don't need to specify the type parameter twice. The second time you can just say <> and trust the compiler to figure it out.

PersonObjects is a plural name for a singular thing. So we can just make that singular. And there's not much point in saying that it is an object (which it actually isn't, it's a class). Every instantiation of a class in Java is an object. It could be just Person.

Similarly, listOfPeopleObjects could be just people. There's some controversy over the list. There is a notation called Hungarian notation where the type is included in the name. I personally prefer to just use plural names for collections of objects. But others disagree.

Clear prompts

                System.out.println("nDo you want to quit and see list? Enter 1 for yes and 0 for non");

Why do you want to indicate yes? Or no? You want to indicate yes to quit and no to continue. So why say yes and no? Consider

                System.out.println("nDo you want to quit and see list? Enter 1 to quit or 0 to continuen");

Now we don't have to figure out what yes and no mean in this context (e.g. you could have been asking "Do you want to continue?" with everything else the same.

do/while

You may want to read up on the do/while syntax. That would allow you to avoid asking for an initial response. I.e. you wouldn't have to enter anything to start the processing. Only to continue afterwards.

It would disrupt your current nextInt/nextLine pattern though. You might consider how you could make that work. Hint: a custom method could do both commands in one unit of work.

PeopleObjects is an odd name for a couple reasons:

• Both words are plural, even though each instance only represents a single person.


• Having "object" in the name is a little redundant. It's implied that the class is representing an object.

I'd just call the class Person.

And you didn't give PeopleObjects a constructor. You're constructing an instance of the class ahead of time, then asking for input and calling the setters. What if you forgot to set the birthday or name? They would be left as null and potentially cause problems later. If properties are required (which both name and birthday would presumably be), have them supplied by a constructor:

public Person(String name, String birthDay) {

    this.name = name;

    this.birthDay = birthDay;

}



. . .



List<Person> people = new ArrayList<Person>();



Scanner input = new Scanner(System.in);



System.out.println("Enter any number other than 1 to start program."+"n");



while (input.nextInt() != 1){

        // This is needed since nextInt doesn't consume the newline

        input.nextLine();



        System.out.println("n###What is the name of the person?###n");

        String name = input.nextLine();



        System.out.println("n###What is their birthday?###n");

        String birthday = input.nextLine();



        Person person = new Person(name, birthday);



        people.add(person);



        System.out.println("nDo you want to quit and see list? Enter 1 for yes and 0 for non");

}

I changed a few things about that loop:

• As mentioned above, the person isn't constructed until the necessary data is already obtained, and the data is passed directly to the constructor.




• userChoice was never used. It was just storing the users entry, but that choice was only ever used in the condition. If you really wanted it for debugging purposes, it may be appropriate, but I didn't think that it was necessary here.




• I renamed the list of people to simply people. This was replacing your PeopleObjects people = new PeopleObjects(); line. Again, calling a single person people is confusing. I also changed the type of people to List, from ArrayList. Always use the broadest type possible. In this case, you don't need any operations specific to ArrayList; you're only using List methods. To see why this is important, give this a read over. Note the example used in the question.

Your other loop at the bottom is sub-optimal as well. First, you're constantly writing people.get(i). For an ArrayList, get is very fast, but those calls are causing unneeded bloat. Create an intermediate variable person in the loop to neaten things up:

for(int i = 0; i < people.size(); i++) {

    Person person = people.get(i);

    String name = person.getName();



    if(name.equals("Joe")) {

        System.out.println("Happy Birthday Joe! You are special! Your birthday is on "

                           + person.getBirthDay() + " n");



    } else if(i % 2 == 0) {

        System.out.println("Happy Birthday to " + name + ".n");



    } else {

        System.out.println("Happy Birthday to " + name + " too!!!n");

    }

}

I ended up creating a name variable as well since the name of the person was needed in multiple places.

You also had inconsistent spacing that I corrected. Some places had everything all smooshed together, and then other places (like with ==) had spacing. I added spaces around all the operators, then broke up one of the longer lines. I also added empty lines after each println call so the cases were a little more distinct.

Unless you need the i % 2 == 0 check to give a special message to every arbitrary second person, this loop could be slightly simplified by using an enhanced for-loop that doesn't rely on indices:

// Read as "for each person in people"

for(Person person : people) {

    String name = person.getName();



    if(name.equals("Joe")) {

        System.out.println("Happy Birthday Joe! You are special! Your birthday is on "

                           + person.getBirthDay() + " n");



    } else {

        System.out.println("Happy Birthday to " + name + " too!!!n");

    }

}