User input happy birthday program












5












$begingroup$


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
}
}









share|improve this question







New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$












  • $begingroup$
    Welcome to code review. This looks interesting to me. Hope you learn a lot here.
    $endgroup$
    – 422_unprocessable_entity
    7 hours ago
















5












$begingroup$


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
}
}









share|improve this question







New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$












  • $begingroup$
    Welcome to code review. This looks interesting to me. Hope you learn a lot here.
    $endgroup$
    – 422_unprocessable_entity
    7 hours ago














5












5








5





$begingroup$


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
}
}









share|improve this question







New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$




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
}
}






java beginner array homework






share|improve this question







New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











share|improve this question







New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









share|improve this question




share|improve this question






New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









asked 8 hours ago









Steven HalversonSteven Halverson

262




262




New contributor




Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





New contributor





Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






Steven Halverson is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.












  • $begingroup$
    Welcome to code review. This looks interesting to me. Hope you learn a lot here.
    $endgroup$
    – 422_unprocessable_entity
    7 hours ago


















  • $begingroup$
    Welcome to code review. This looks interesting to me. Hope you learn a lot here.
    $endgroup$
    – 422_unprocessable_entity
    7 hours ago
















$begingroup$
Welcome to code review. This looks interesting to me. Hope you learn a lot here.
$endgroup$
– 422_unprocessable_entity
7 hours ago




$begingroup$
Welcome to code review. This looks interesting to me. Hope you learn a lot here.
$endgroup$
– 422_unprocessable_entity
7 hours ago










2 Answers
2






active

oldest

votes


















3












$begingroup$

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.






share|improve this answer









$endgroup$





















    3












    $begingroup$

    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");
    }
    }





    share|improve this answer











    $endgroup$













      Your Answer





      StackExchange.ifUsing("editor", function () {
      return StackExchange.using("mathjaxEditing", function () {
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      });
      });
      }, "mathjax-editing");

      StackExchange.ifUsing("editor", function () {
      StackExchange.using("externalEditor", function () {
      StackExchange.using("snippets", function () {
      StackExchange.snippets.init();
      });
      });
      }, "code-snippets");

      StackExchange.ready(function() {
      var channelOptions = {
      tags: "".split(" "),
      id: "196"
      };
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function() {
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled) {
      StackExchange.using("snippets", function() {
      createEditor();
      });
      }
      else {
      createEditor();
      }
      });

      function createEditor() {
      StackExchange.prepareEditor({
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader: {
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      },
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      });


      }
      });






      Steven Halverson is a new contributor. Be nice, and check out our Code of Conduct.










      draft saved

      draft discarded


















      StackExchange.ready(
      function () {
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f214120%2fuser-input-happy-birthday-program%23new-answer', 'question_page');
      }
      );

      Post as a guest















      Required, but never shown

























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      3












      $begingroup$

      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.






      share|improve this answer









      $endgroup$


















        3












        $begingroup$

        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.






        share|improve this answer









        $endgroup$
















          3












          3








          3





          $begingroup$

          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.






          share|improve this answer









          $endgroup$



          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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered 7 hours ago









          mdfst13mdfst13

          17.8k62157




          17.8k62157

























              3












              $begingroup$

              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");
              }
              }





              share|improve this answer











              $endgroup$


















                3












                $begingroup$

                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");
                }
                }





                share|improve this answer











                $endgroup$
















                  3












                  3








                  3





                  $begingroup$

                  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");
                  }
                  }





                  share|improve this answer











                  $endgroup$



                  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");
                  }
                  }






                  share|improve this answer














                  share|improve this answer



                  share|improve this answer








                  edited 6 hours ago

























                  answered 7 hours ago









                  CarcigenicateCarcigenicate

                  3,44711631




                  3,44711631






















                      Steven Halverson is a new contributor. Be nice, and check out our Code of Conduct.










                      draft saved

                      draft discarded


















                      Steven Halverson is a new contributor. Be nice, and check out our Code of Conduct.













                      Steven Halverson is a new contributor. Be nice, and check out our Code of Conduct.












                      Steven Halverson is a new contributor. Be nice, and check out our Code of Conduct.
















                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function () {
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f214120%2fuser-input-happy-birthday-program%23new-answer', 'question_page');
                      }
                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      How to label and detect the document text images

                      Vallis Paradisi

                      Tabula Rosettana