Practicing OOP with Student and Person

Question

I've just learned the OOP concept of abstraction, and I was wondering if I was implementing it correctly in my code and if you guys can point out any bad practices.

test1.java

package OOPConcepts;

import java.util.Scanner;
public class test1 {
    static public Scanner input = new Scanner(System.in);
    public static void main(String[] args) {
        String fName, lName;
        int age;
        Student newPerson = new Student();
        input.close();
    }
}

Person.java

package OOPConcepts;

public class Person
{
    static private int pNumber = 0;
    private int pID;
    public int age;
    private String type;
    public String fName, lName;

    Person()
    {
        pNumber++;
        pID = pNumber;

        System.out.println("Enter first name:");
        String fName = test1.input.next();
        System.out.println("Enter last name:");
        String lName = test1.input.next();
        System.out.println("Enter age:");
        int age = test1.input.nextInt();
        type = "person";
        describe(fName, lName, age);

        System.out.println("Made a " + type);
    }
    public void setType(String type){
        this.type = type;
    }
    public String getType()
    {
        return this.type;
    }
    public void describe(String fName, String lName, int age)
    {
        this.fName = fName;
        this.lName = lName;
        this.age = age;
        System.out.println("Name: " + this.fName + " " + this.lName + "|Age: " + this.age);
    }
}

Student.java

package OOPConcepts;

public class Student extends Person{
    static private int sNumber = 0;
    private int studentID;
    public String major;

    Student()
    {
        sNumber++;
        studentID = sNumber;
        setType("student");

        System.out.println("Who is also a " + getType());
        System.out.println("Enter major: ");
        String major = test1.input.next();
        this.major = major;
    }
    public void setStudentID(int idNumber)
    {
        this.studentID = idNumber;
    }
    public int getStudentID()
    {
        return this.studentID;
    }
}

Answer 1

What is OOP?

There is more to OOP than just class inheritance and your example shows painfully clearly why class hierarchy is almost always completely unable to represent the real world. If a student is a specialized type of a person, then a person can not be a student and a teacher at the same time (e.g. an assistant to a teaching professor). Also, that person can no longer be a part time employee at the cafeteria either, because the person's status has been locked into being a student and student only. Clearly, class inheritance is the wrong tool here. The relation should be composition:

public class Student {
    private final StudentId studentId;
    private final Person person;

    public Student(StudentId studentId, Person person) {
        this.studentId = studentId;
        this.person = person;
    }

    ...
}

Answer 2

Sorry, you are not implementing OOP well, since it is true that you are defining a class, declaring its fields and instantiating it, this is being done in a wrong way.

The Good

• The abstraction is carried out properly for an exercise (because in a real design you may need more details)
• The idea of giving a person an ID aka pNumber is something which will be closely related to ORM (Object Relational Mappings) which is a way to communicate your database(s) with your object oriented design

Review

• Don't Declare Public Variables, you have Getter/Setter methods to access the values or modify them.
• In the class which owns your main method is where should go all the data gathering. Doing this

    Person()
    {
        pNumber++;
        pID = pNumber;
        //...

Is a bad practice you should (must) untie the logic from the view. Even if this is concept from the MVC (model view controller) pattern it is important to do that in each project in order to improve software quality, reduce errors and having a independent work structure so that when you change the UI for the user it won't affect the logic.

The alternative is to create a parametrized constructor:

    Person(String fName, String lname, int age)
    {
        //also conssider call pID as personId (be descriptive)
        this.pID = ++pNumber;
        this.fName = fName;
        this.lName = lName;
        this.age = age;
    }

///how to use (in your main method)
    Person person = new Person("Syntëy", "Emyl", 27);

• The variable type in Person class is something ambiguous (yes it is the type of the person but which implications does it have? which values may it take?), you should add a comment before the variable to describe it.

Some suggestions

• Always use private fields.
• The names given to your variables, it is quite short, it is not bad but it is better to specify them well so that your getter/setter have a quick meaning, fName, lName could be changed by firstName, lastName.
• The static variable pNumber or sNumber could be named personIdCounter studentIdCounter
• It is not a standard but most people works with a regular expression which matches the following: [AccessModifier] [DataType] [FieldName]; and for static variables [AccessModifier] static [DataType] [FieldName];
• Try to not declare multiple fields (attributes) in the same line as in public String fName, lName;

Finally, I hope it has been helpful.

Answer 3

Miguel has made some excellent points; I won’t repeat those. Here are some more comments / suggestions:

---

The type field is effectively public, because you have public void setType(String type), yet it is clear that this field should not be settable externally. It shouldn’t exist at all, since we can simply ask:

if (person instanceof Student) {…}

instead of needing to check a string to see if it is equal to "student" (or was that "Student"? Or maybe it was "STUDENT"??? Maybe you should be using named constants ... or better ... an enum!)

---

age is a horrible field. As time goes by, it changes. If you have multiple people in a database, you are constantly updating the records. Store their birthdate, and compute their age.