3
\$\begingroup\$

Problem Statement:

Design a STUDENT class to store roll, name, course, admission date and marks in 5 subjects taken from user. Create an array of STUDENT objects. Provide methods corresponding to admission date and receiving marks, preparing mark sheet. Support must be there to show the number of students who have taken admission.

What should I do to improve the error handling features?

I am looking for code review, best practices and better approaches.

Student class:

package student;

import java.util.Date;
import java.util.Scanner;

class Student {
    public String name, course;
    public int marks[] = new int[5];
    public int roll;
    public String adm_date;
    public static int student_count = 0;;

    public static void student_number()
    {
        System.out.println("Number of students admitted : " + student_count);
    }

    public int ret_roll() {
        return roll;
    }

    public void admission() {
        Scanner sc = new Scanner(System.in);
        Date date = new Date();
        System.out.println("STUDENT  DETAILS\n");
        System.out.println("\nEnter name: ");
        name = sc.nextLine();
        System.out.println("\nEnter course name: ");
        course = sc.nextLine();
        adm_date = date.toString();
        System.out.println("Admission Date:" + adm_date);
        student_count++;
        roll = student_count;
        System.out.println("Roll: "+ roll);

    }

    void get_marks() {
        Scanner sc = new Scanner(System.in);
        boolean flag;
        for (int i = 0; i < 5;) {
            flag = true;
            do{
                if(!flag){
                    System.out.println("Wrong input.");
                }
                System.out.println("Enter marks in subject " + (i + 1) + ":");
                marks[i] = sc.nextInt();

                flag = false;
            }while((marks[i] > 100) || (marks[i] < 0) );

            i++;

        }


    }

    void marksheet() {
        int j;
        System.out.println("STUDENT DETAILS");
        System.out.println("NAME               : " + name);
        System.out.println("ROLL NUMBER        : " + roll);
        System.out.println("COURSE             : " + course);
        System.out.println("ADMISSION DATE     : " + adm_date);
        for (j = 0; j < 5; j++){
            System.out
                    .println("MARKS IN SUBJECT " + (j + 1) + " : " + marks[j]);
        }
    }

    String get_name() {
        return name;
    }

    String get_admission_date() {
        return adm_date;
    }
};

The Student list class to work with the student class.

StudentList class:

package student;

import java.util.*;

public class StudentList {
public static void main(String[] args) {
    Scanner sc = new Scanner(System.in);
    int size;
    System.out.println("\nEnter the maximum number of student:");
    size = sc.nextInt();
    int i = 0;
    boolean flag = false;
    Student list[] = new Student[size];
    char choice;
    while (!flag) {
        System.out
                .println("\n\n*******************STUDENTS ADMISSION********************\n");
        System.out.println("1. Admission");
        System.out.println("2. Enter marks");
        System.out.println("3. Display marksheet");
        System.out.println("4. Total number of students");
        System.out.println("6. EXIT");
        System.out.println("Enter choice: ");
        choice = sc.next().charAt(0);
        switch (choice) {
        case '1':
            if (i >= size) {
                System.out.println("Maximum student capacity reached");
                break;
            }
            list[i] = new Student();
            list[i].admission();
            list[i].get_marks();
            i++;
            break;
        case '2': {
            int j, k;
            System.out
                    .println("Enter the student roll number whose marksheet is to be prepared: ");
            k = sc.nextInt();
            for (j = 0; j < Student.student_count; j++) {
                if (list[j].ret_roll() == k) {
                    list[j].get_marks();
                    break;
                }
            }
            if (j == Student.student_count)
                System.out.println("Student not admitted yet\n");
            break;
        }
        case '3': {
            int j, k;
            System.out
                    .println("Enter the student roll number whose marksheet is to be displayed: ");
            k = sc.nextInt();
            for (j = 0; j < Student.student_count; j++) {
                if (list[j].ret_roll() == k) {
                    list[j].marksheet();
                    break;
                }
            }
            if (j == Student.student_count)
                System.out.println("Student not admitted yet\n");

            break;
        }
        case '4':
            System.out.println("Total students: ");
            Student.student_number();
            break;
        case '6':
            System.out.println("Thank You");
            flag = true;
            break;
        default:
            System.out.println("\nInvalid choice");
        }
    }
    sc.close();
}

}
\$\endgroup\$
  • \$\begingroup\$ What does roll mean? \$\endgroup\$ – Pimgd Nov 4 '14 at 11:09
  • \$\begingroup\$ Each student is assigned a unique number(i.e, roll number). In my implementation the roll number is the (number of students before that particular student was admitted)+1 \$\endgroup\$ – SouvikMaji Nov 4 '14 at 11:13
  • \$\begingroup\$ @rick112358 You mean enrollment number then. \$\endgroup\$ – Erfan Nov 4 '14 at 11:27
5
\$\begingroup\$

1. Best practices

The details of the student i.e the instance variables: Name, Course , Marks etc should be private and you should provide public methods to operate on them. Instance variables are made private to force the users of those class to use methods to access them. In most cases there are plain getters and setters but other methods might be used as well.

Using methods would allow you, for instance, to restrict access to read only, i.e. a field might be read but not written, if there's no setter. That would not be possible if the field was public.

Say for example, your roll number field is public.To use it one can do something like this -> student.roll=123123

Making your roll number private and providing getter and setters can help you a lot in the long run

private int roll;

public void setRoll(int roll){
   this.roll=roll
}

suppose in the future need arises to perform validation while setting roll no , your decision to make the roll no field private and provide setters will help you a lot. You could just do ->

public void setRoll(int roll){
    performvalidation();
    this.roll=roll;
}

This would not have been possible if you had public fields since clients would have set the roll number using object.roll=roll number. This would break their code too.

2.Exception Handling Scenarios

you are using size = sc.nextInt();

what if user inputs a character by mistake? How would you handle that? use something like

String tempSize=sc.next
validateIfEnteredTextIsInteger
int size=Integer.parseInt(tempSize);  

Another thing , in the 3rd case of your switch statement in the StudentList class, you loop over every student till you find the one with the matching roll no.Since the roll numbers are unique , a Map can come in handy.

Map<Integer,Student> students = new HashMap<>();
Student student=new Student();

student.setRoll(rollnumber);
Map.put(rollNumber,student);

Doing this , in your 3rd case , you could simply do

Student student=students.get(rollNumber);
student.get_Marks();
\$\endgroup\$
  • \$\begingroup\$ How does validateIfEnteredTextIsInteger works?Please explain. Should I use BufferedReader with Integer.parseInt() instead of Scanner? \$\endgroup\$ – SouvikMaji Nov 4 '14 at 14:28
  • \$\begingroup\$ Scanner is fine. You can use regex for example to check whether a certain string is a number. For example String.matches("[0-9]*") .For regex tutorials see regexone.com \$\endgroup\$ – Ishan Soni Nov 4 '14 at 15:26
5
\$\begingroup\$

Naming

  • in Java, it is customary to use camelCase, not snake_case
  • roll isn't a very good name. enrollmentNumber or just id would be better
  • in general, don't shorten words in variable names. adm for example could be a lot of things, admission would be a lot clearer.
  • ret_roll should be getRoll. Getters (methods which return the field of an object) are always named like this: getFieldName.
  • flag is quite generic. A flag for what?
  • StudentList isn't really a list, it's just a class containing a main method (which in turn contains an array).

static Student count

Using a static field like student_count like this is bad practice. I would create a Counter class, which could look something like this:

public class Counter {

    int currentCount = -1;

    public int getNewId() {
        currentCount++;
        return currentCount;
    }

    public int getCurrentCount() {
        return currentCount;
    }

}

General Structure

Your student class does too many things. It reads input, it reads output, it counts students, and it holds and handles student data. You should think about creating new classes for at least the first three functionalities. Ideally, I would gather user input outside the class and pass it via constructor/methods to the class.

Misc

  • get rid of additional semicolons (;;)
  • define variables where they are needed (eg declare j inside the loop)
  • be consistent with your placement of {
  • assign variables right when they are declared (eg int size = sc.nextInt(); instead of adding an extra line)
  • your case 2 and case 3 in StudentList contain a lot of duplicate code, try to extract it to a function
  • this for loop: for (int i = 0; i < 5;) is odd. You placed the i++ somewhere else, why? Just put it inside the loop: for (int i = 0; i < 5; i++)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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