Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have made a student details project. The user is asked to enter the info for two students, then is displayed on the screen. The question that I have is 1: Have I used inheritance correctly and 2: What would be a better way of outputting the results.

My Code:

 public class Person extends Test
    {
        // Variables

            String name;
            int age;
            String address;

            // Default Constructor

            Person()
            {
                name = "";
                age = 0;
                address = "";
            }

            Person(String name, int age, String address)
            {
                this.name = name;
                this.age = age;
                this.address = address;

            }

            String getName()
            {
                return name;
            }


            public void display()
            {

                System.out.println("Name = "+ name);
                System.out.println("Age = "+ age);
                System.out.println("address = "+ address);



            }

    }


public class Student extends Person
{

    int studentNum, semester;

    Student(String name, int age, String address, int studentNum, int semester)
    {
        super(name, age, address); // calls parent class’s constructor 
        this.studentNum = studentNum;
        this.semester = semester;
        //this.course = course;

    }

    public String getName() // name
    {
        return name;
    }

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

    public int getAge() // age
    {
        return age;
    }

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

    public String getAddress() // address
    {
        return address;
    }

    public void setAddress(String address) 
    {
        this.address = address;
    }

    public int getStudentNum() // studentNum
    {
    return studentNum;
    }

    public void setStudentNum(int studentNum) 
    {
        this.studentNum = studentNum;
    }

    public int getSemester() // semester
    {
    return semester;
    }

    public void setSemester(int semester) 
    {
        this.semester = semester;
    }





    void Display() // Method Overriding
    {



    }





}



public class Course extends Student
{
    String course;

    Course(String name, int age, String address, int studentNum, int semester, String course) 
    {
        super(name, age, address, studentNum, semester);
        this.course = course;


    }

    public void display()
    {       

    }

}

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;

public class Test  implements StudentFees
{
    public static void main(String args[ ]) throws IOException
    {
        System.out.println("============" + "=================");
        System.out.println("Students " + "Personal Details");
        System.out.println("============" + "=================");

        String name, address, course;
        int age, studentNum, semester;

        List<Student> studentsList = new ArrayList<Student>(); // array list to store user input



        for (int i = 0; i < 2; i++) 
        {

        int studentNumber = (i + 1);

        //System.out.println("");
        //System.out.println("Please enter " + "data for student " + studentNumber);

        InputStreamReader converter = new InputStreamReader(System.in);
        BufferedReader in = new BufferedReader(converter);

        System.out.println("Enter Student "+ studentNumber + " Name:"); 
        name = in.readLine();

        System.out.println("Enter Student " + studentNumber + " Age (Integer):");
        age = Integer.valueOf(in.readLine());

        System.out.println("Enter Student " + studentNumber + " Address:");
        address = in.readLine();

        System.out.println("Enter Student " + studentNumber + "  Number:"); 
        studentNum = Integer.valueOf(in.readLine());

        System.out.println("Enter Student " + studentNumber + " Semester:");
        semester = Integer.valueOf(in.readLine());

        System.out.println("Enter Student " + studentNumber + " Course:");
        course = in.readLine();


        Student student = new Student(name, age, address, studentNum, studentNum);

        studentsList.add(student); // add student
        }

        for (int j = 0; j < studentsList.size(); j++)
        {

            Student st = studentsList.get(j);

            System.out.println("Information of Student : " + (j + 1)); 
            System.out.println("");
            System.out.println("Name: " + st.getName() + " - Age: "+st.getAge() + " - Address: " + st.getAddress() + " - Student Number: " + st.getStudentNum() + " - Semester: " + st.getSemester() +  " - Course: " + st.getCourse()); // print out results entered by user
            System.out.println("");
        }

    }

    String course;

    public String getCourse() 
    {
        return course;
    }

    public void setCourse(String course) 
    {
        this.course = course;
    }

    public void payFees(float fees) 
    {

    }

}
share|improve this question
3  
Course definitely should not extend Student. Inheritance represents an is-a relationship, and a Course is not a Student. –  S.L. Barth Dec 9 '12 at 8:43
    
Ok should I make any new classes like EveningCourse extends course? –  David Needham Dec 9 '12 at 14:33
    
If you need a new class EveningCourse, it should extend Course. Don't make them until you need them. –  S.L. Barth Dec 11 '12 at 9:23
add comment

4 Answers

You must remember that inheritance is good to avoid code duplication, but also you must know that you will have messy code if you will extends for one field or method, they must be connected logically Student extends Person it's OK, Cat extends Dog isn't OK. I think you don't need make Person extends Test.

share|improve this answer
add comment

There is an approach: prefer composition instead of inheritance.

Inherit only and only when you need to "avoid copy-paste" or use the same behaviour(the same method) and extend them.

Student is a Person - ok. Course have few Students - it is a composition(Student should be a field in class Course, and will be collection of students - List), not a inheritance.

Why you did " public class Person extends Test" is unclear for me, can not explain it at all.

share|improve this answer
add comment

Talk yourself through your "problem domain" and write it as you go. Say a little, write a little. The details will come along. Don't worry about them all at once. Make sure the little you write each time is right first.

For example

Well, there's a Person

public Class Person {}

Is a Student a Person? Yes. Is a Person a student? Well, sometimes.

public Class Student extends Person {}

What's the basic stuff of a Person/Student? name, age, address, studentID, semester....

public class Student extends Person () {
    public string Name;
    public string Age;
    public string Address;
    public string ID;
    public string semester;
}

Oh, some of those belong to all Persons, some are students only

public class Person () {
        public string Name;
        public string Age;
        public string Address;
}

public class Student extends Person () {
        public string ID;
        public string semester;
}

Students take courses. I need a course class. Is a course a student? no. vice versa? no. Oh - a student takes a course. Or a student has courses - has means composition not inheritance

public class Student extends Person () {
            public string ID;
            public string semester;
            ArrayList courses;
 }
 public class Course {} // don't know what's inside this yet.

OK, You get the idea.

Displaying

Your basic idea of making strings of "label: value" is the right idea, but implemented wrong. And once implemented - use it!!

All objects have a toString() method. Override it. Then all you have to "say" to use it is System.out.println(someStudent.toString()) BAM! object oriented programming. Also, DO NOT use println in your toString(). toString() should build all the object information into a single formatted string and pass that. Then you use it as shown.

// in the Person class
public override string toString(){
   string me = "";
   me += "Name: " + this.Name + "\n"  //new line
   me += "Age: " + this.Age + "\n";
   me += "Address: " + this.Address + "\n";
   return me;
}

Now, what class would this go into? Person. What about Student? The above already does most of what we need for Student too. Do we duplicate it in Student class? NO! Object Oriented Design to the rescue!

// in the Student class
public string toString(){
    string me = "";
    me += base.toString();
    me += "semester: " + this.semester + "\n";
    ......
    return me;
}
share|improve this answer
add comment

Already said: Course should probably not extend a student. Always ask yourself: Is subclass a special thing of a parentclass?

You could add a List<Course> courses attribute to Student and a List<Student> students attribute to Course (Both if n:m, only one if 1:n).

Depending on the requirements, you could override the toString() method, then you could write print(student) instead of all the concatenations.

Try to have a look at the Scanner class and some examples. It makes command line reading much easier.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.