HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple input/output practice

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inputsimplepracticeoutput

Problem

This code is just some exercise code printing questions and waiting for answers. It is working perfectly well as far as I can tell. I would like to know if this code could be considered acceptable or how it could be improved.

```
import java.util.ArrayList;
import java.util.Scanner;

public class MainClass {

public static void main(String[] args) {

ArrayList list = new ArrayList();
Citizens p1 = new Portuguese();
list.add(p1);
p1.addName();
p1.addAge();
p1.addAdress();
Citizens p2 = new German();
list.add(p2);
p2.addName();
p2.addAge();
p2.addAdress();

boolean addCitizen = true;
while (addCitizen) {
Scanner input = new Scanner(System.in);
System.out.println("Do you want to add another citizen?Y or N?");
String answer = input.next();
if (answer.equals("y")) {
System.out.println("Do you want to add a Portuguese or a German citizen?PT or GER?");
answer = input.next();
if (answer.equals("pt")) {
Citizens p3 = new Portuguese();
list.add(p3);
p3.addName();
p3.addAge();
p3.addAdress();
} else if (answer.equals("ger")) {
Citizens p3 = new German();
list.add(p3);
p3.addName();
p3.addAge();
p3.addAdress();
} else if (!answer.equals("pt") || (!answer.equals("ger"))) {
System.out.println("Please choose PT or GER!");
}
} else if (answer.equals("n")) {
System.out.println("You´re not gonna add a new citizen!");
addCitizen = false;

} else {
System.out.println("Please enter y or n");
}

}
System.out.println("The end");
}

Solution

Naming

Naming is fundamental to make your code easier to read.

ArrayList list = new ArrayList();


Name this to citizens.

Citizens p1 = new Portuguese();

p1 is not an adequate name. Rather name it as person1. Replace also p2 to person2.

Rename Citizens to Citizen: person1 is Citizen not Citizens. Same thing for person2.

Also use correct English spelling and follow Java capitalization conventions when you name a variable:

boolean afirmativeanswer = true;


should be affirmativeAnswer.

Java is a language where CamelCase is mostly respected. Why didn't you use it?

Method

-
A constructor is mostly used to access that classes methods and attributes parallel to other objects created.

-
For an infinite-loop, you can do while(true) (preferred) or for(;;). Use break to exit from the while loop. So you don't even need to use afirmativeanswer.

-
Use toLowerCase() method to not differentiate from inputting UPPERCASE or lowercase characters.

-
There is no need to make answer and answernr instance variables. Just put them into the methods when you need them.

Code Snippets

ArrayList<Citizens> list = new ArrayList<Citizens>();
boolean afirmativeanswer = true;

Context

StackExchange Code Review Q#111844, answer score: 4

Revisions (0)

No revisions yet.