Simple input/output practice

Question

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<Citizens> list = new ArrayList<Citizens>();
        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"); 
 }
}

class Citizens

import java.util.Scanner;

public class Citizens {

    private String name;
    private int age;
    private String adress;
    String answer;
    public int answernr;
    boolean afirmativeanswer = true;
    Scanner input = new Scanner(System.in);

    public void addName() {

        System.out.println("Do you want to add the citizen name?Y or N?");
        answer = input.nextLine();
        while (!answer.equals("y") || (!answer.equals("n"))) {

            if (answer.equals("y")) {
                System.out.println("Please give the citizen´s name !");
                String giveName = input.nextLine();
                this.setName(giveName);
                break;
            } else if (answer.equals("n")) {
                System.out.println("Not adding a name!");
                break;
            } else {
                System.out.println("Please enter y or n");
                answer = input.nextLine();
            }
        }

    }

    public void addAge() {

        System.out.println("Do you want to add " + this.getName() + "´s age? Y or N?");
        while (afirmativeanswer) {
            answer = input.nextLine();

            if (answer.equals("y")) {
                System.out.println("Please enter " + this.getName() + "´s age!");
                answernr = input.nextInt();
                this.setAge(answernr);
                afirmativeanswer = false;
            } else if (answer.equals("n")) {
                System.out.println("You choose not to add " + this.getName() + "´s age!");
                afirmativeanswer = false;
            } else {
                System.out.println("Please enter y or n");
            }
        }
    }

     public void addAdress() {
            System.out.println("Do you want to add " + this.getName() + "´s Adress?Y or N?");

            afirmativeanswer = true;

            while (afirmativeanswer) {
              answer =  input.next();
                if (answer.equals("y")) {
                    System.out.println("Enter " + this.getName() + "´s Adress!");
                    answer = input.next();
                    this.setAdress(answer);
                    afirmativeanswer = false;
                } else if (answer.equals("n")) {
                    System.out.println("You choose not to add " + this.getName() + "´s Adress!");
                    afirmativeanswer = false;
                } else {
                    System.out.println("Please choose Y or N!");

                }
            }

        }

    public void setName(String s) {
        this.name = s;
    }

    public String getName() {
        return name;
    }

    public void setAge(int i) {
        this.age = i;
    }

    public int getAge() {
        return age;
    }

    public void setAdress(String c) {
        this.adress = c;
    }

    public String getAdress() {
        return adress;
    }

    public String toString() {
        return name;
    }

}

The Portuguese and German classes have just one constructor each.

Answer 1

Naming

Naming is fundamental to make your code easier to read.

ArrayList<Citizens> list = new ArrayList<Citizens>();

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.

Answer 2

A few casual observations:

• Do you need that huge gap in code in your MainClass?
• You can also change your prompts, some are kind of misleading such as System.out.println("Do you want to add another citizen?Y or N?"); which prompts for capitols, but accepts lower case y , n. Or accept lower and upper cases. Clarify all the prompts to show what kind of input Is expected.

Change these lines

public void addName() {

    System.out.println("Do you want to add the citizen name?Y or N?");
    answer = input.nextLine();
    while (!answer.equals("y") || (!answer.equals("n"))) {

        if (answer.equals("y")) {
            System.out.println("Please give the citizen´s name !");
            String giveName = input.nextLine();
            this.setName(giveName);
            break;
        } else if (answer.equals("n")) {
            System.out.println("Not adding a name!");
            break;
        } else {
            System.out.println("Please enter y or n");
            answer = input.nextLine();
        }
    }

}

to this:

public void addName() {

do {
    System.out.println("Do you want to add the citizen name ? y or no?");
    answer = input.nextLine();
    if (answer.equals("y")) {
        System.out.println("Please give the citizen´s name !");
        String giveName = input.nextLine();
        this.setName(giveName);
        break;
    } else if (answer.equals("n")) {
        System.out.println("Not adding a name!");
        break;
    } else { //optional if you want to ask please
        System.out.println("Please enter y or n");
        answer = input.nextLine();
    }    
}
}

I wasn't able to double check this, and am unsure if your original while loop worked as intended. As it looks to me that it would run for any input that's not "y" or "n", so if you input "N" it would run. Which I doubt is want you wanted.

Answer 3

Using value objects

Instances of the Citizen (rightly pointed out by @DrProgrammer that its name should be singular) class are also known as value objects, in which they should only contain the required values with minimal logic in it. This particular 'feature' makes them agnostic to the environment they are used in: for example, should System.in still be referenced if a user is using a web-based GUI?

As such, the methods addName(), addAge() and addAddress() are better suited in your MainClass (Main?) class, such that they return the desired values for creating a Citizen object. Furthermore, having multiple instances of Scanner on a single System.in InputStream is usually not recommended, as invoking close() on any of them prevents others from reading more input, and generally speaking it's not a particularly efficient way of managing an I/O resource.

Referencing objects

It's often easier to start creating a single variable to reference an object, which in your case is p1. Then, you need another, so p2 it is. Next, you have a p3, and it starts getting out of control have to manage pN variables running around your codebase. The question here is, do you need to refer to all of them at once?

Taking a step back, is it indicated to the user at any time that they are entering the details of a Portuguese citizen first, then a German one? Does this ordering matter?

A more straightforward approach to adding a Citizen to your List can be the following pseudo-code:

// assuming we want at least one entry, else use a while-loop to prompt first
do {
    citizenship = getCitizenship
    name = getName
    age = getAge
    address = getAddress
    if (citizenship == Portuguese) {
        list.add(new Portuguese(name, age, address));
    } else if (citizenship == German) {
        list.add(new German(name, age, address));
    }
} while (promptForNextCitizenEntry)

Enumerating types

Since you are only handling for Portuguese and German citizenship, you can leverage on Java's enum type to define them as constants. The benefits of this approach are that you have a simpler way of validating citizenship, comparing them or even introducing new ones.

Putting into code

Citizen will have a constructor that accepts the three required values:

public class Citizen {

    private String name;
    private int age;
    private String address;

    public Citizen(String name, int age, String address) {
        this.name = Objects.requireNonNull(name);
        this.age = age;
        this.address = Objects.requireNonNull(address);
    }

    // ...

}

You can introduce the Citizenship enum:

enum Citizenship {
    PT, GER;

    public static Citizenship get(String input)) {
        try {
            // toUpperCase() as the input must match the case of the values
            return Citizenship.valueOf(input.toUpperCase());
        } catch (IllegalArgumentException e) {
            return null;
        }
    }

    public static String list() {
        return Arrays.toString(values());
    }
}

Your MainClass should look something like the following (with the aid of try-with-resources for efficient handling of I/O resources):

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        // interface over implementation variable declaration,
        // and type inference for generic instance creation ('<>')
        List<Citizen> citizens = new ArrayList<>();
        do {
            Citizenship citizenship = getCitizenship(scanner);
            String name = getString(scanner);
            int age = getInteger(scanner);
            String address = getString(scanner);
            if (citizenship == Citizenship.PT) {
                citizens.add(new Portuguese(name, age, address));
            } else if (citizenship == Citizenship.GER) {
                citizens.add(new German(name, age, address));
            } else {
                // will not reach here due to the validation done above
            }
        } while (hasNextCitizen(scanner));
        // do something with citizens
    }
}

private static boolean hasNextCitizen(Scanner scanner) {
    System.out.println("Enter another citizen (true/false)? ");
    while (!scanner.hasNextBoolean()) {
        System.out.println("Please enter 'true' or 'false' without quotes.");
    }
    return scanner.nextBoolean();
}

private static String getString(Scanner scanner) {
    // similar implementation to above
}

private static int getInteger(Scanner scanner) {
    // similar implementation to above
}

private static Citizenship getCitizenship(Scanner scanner) {
    System.out.println("Enter citizenship " + Citizenship.list() + ": ");
    Citizenship result = null;
    while (!scanner.hasNext() || (result = Citizenship.get(scanner.next())) == null) {
        System.out.println("Please enter one of the following: "
                                + Citizenship.list());
    }
    return result;
}