trjhtr

I've used Abstract classes and static methods wherever they seemed to be convenient. I've serialized the hashmap entry and read/write it from/to a file.

Please review my code and point me to the mistakes I made. How far away am I from writing production quality code? What are the major drawbacks in my code/design?

import java.io.*;
import java.util.*;

public class Main 

 public static void main(String args) 

 int prompt;
 File myfine = new File("std.bat");
 Scanner sc = new Scanner(System.in);
 if (myfine.length() > 0) 

 try (ObjectInputStream ois = new ObjectInputStream(new FileInputStream("std.bat"))) 
 Authentication.uniqueStudent = (HashMap<String, Student>) ois.readObject();
 catch (FileNotFoundException e) 
 e.printStackTrace();
 catch (IOException e) 
 e.printStackTrace();
 catch (ClassNotFoundException e) 
 e.printStackTrace();
 
 

 while (true) 
 System.out.println("If you want to make a fresh entry PRESS 1 n If you want to manipulate data in an existing student entry PRESS 2 " +
 "n If you want to save and quit PRESS 3");
 prompt = sc.nextInt();
 sc.nextLine();

 switch (prompt) 
 case 1:
 Student s1 = new Student();
 System.out.println("Enter the student name");
 String temp = sc.nextLine();
 s1.setName(temp);
 System.out.println("Enter the Student Grade");
 int grade = sc.nextInt();
 s1.setGrade(grade);
 s1.setID();
 Authentication.uniqueStudent.put(s1.getID(), s1);
 System.out.println("The unique student ID is " + s1.getID());
 break;
 case 2:
 System.out.println("Enter the student ID");
 String str = sc.nextLine();

 if (Authentication.uniqueStudent.containsKey(str)) 
 Student objective = Authentication.uniqueStudent.get(str);
 System.out.println("The students name is " + objective.getName() + "nThe students grade is " + objective.getGrade());
 System.out.println("The subjects taken by the student are");
 ArrayList<Courses> c = objective.getCourses();
 if (c != null) 
 for (Courses cr : c) 
 System.out.println(cr.toString());
 
 
 else if(c==null)
 System.out.println("The student has not enrolled in any course ");
 
 System.out.println("The balance in the students account is " + objective.getTotalMoney());

 if (objective.getTotalMoney() >= 800) 
 System.out.println("You have enough money to take " + (objective.getTotalMoney() / 800) + " more courses");

 System.out.println("Would you like to take more courses ? nEnter 1 for yes n" +
 "Enter 2 for no");

 int token = sc.nextInt();
 sc.nextLine();
 if (token == 1) 

 HashSet<Courses> allCourses = new HashSet<>();

 for (Courses cs : Courses.values()) 
 allCourses.add(cs);
 
 if (c != null) allCourses.removeAll(c);
 System.out.println("Select from the following courses");

 for (Courses course : allCourses) 
 System.out.println(course);
 

 String stringu=sc.nextLine();
 Courses crs=null;
 try 
 crs=Courses.valueOf(stringu);
 
 catch (IllegalArgumentException e)
 System.out.println("Wrong Subject entered try again");
 continue;
 

 System.out.println("Debugging "+crs);
 objective.addSubject(crs);
 System.out.println("Now the student has ");
 for(Courses cs: objective.getCourses())
 System.out.println(cs);
 
 

 


 else 
 System.out.println("Invalid Student ID");

 
 break;
 case 3:
 try (ObjectOutputStream ois = new ObjectOutputStream(new FileOutputStream("std.bat"))) 
 ois.writeObject(Authentication.uniqueStudent);
 catch (FileNotFoundException e) 
 e.printStackTrace();
 catch (IOException e) 
 e.printStackTrace();
 

 System.out.println("Saving and Quiting");
 return;

 

 

 



public enum Courses 
 history101,
 mathmatics101,
 english101,
 chemistry101,
 computerscience101;

import java.io.Serializable;
import java.util.ArrayList;


public class Student implements Serializable 

 private String name;
 private String ID;
 private ArrayList<Courses> courses=new ArrayList<>();
 private int totalMoney=3200;

 public int getGrade() 
 return grade;
 

 public void setGrade(int grade) 
 this.grade = grade;
 

 private int grade;

 public String getName() 
 return name;
 

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

 public String getID() 
 return ID;
 

 public void setID() 

 String pick ="abcdefghijklmnopqrstuvwxyz12345";
 String unique = "" + getGrade();
 do 
 unique=""+getGrade();
 for (int i = 0; i < 4; i++) 
 int index = (int) (Math.random() * pick.length());
 unique+=pick.charAt(index);
 
 while (Authentication.uniqueStudent.containsKey(unique));
 this.ID = unique;
 

 public ArrayList<Courses> getCourses() 
 return courses;
 

 public void setCourses(ArrayList<Courses> courses) 
 this.courses = courses;
 

 public int getTotalMoney() 
 return totalMoney;
 

 public void setTotalMoney() 
 this.totalMoney -=800;
 
 public void addSubject(Courses obj)
 setTotalMoney();
 courses.add(obj);
 
 public static void main(String args)


 Student stu=new Student();

 stu.addSubject(Courses.chemistry101);
 

import java.util.HashMap;

public abstract class Authentication 

 public static HashMap<String,Student> uniqueStudent=new HashMap<>();

Some thoughts:

1. The class containing the main method is usually named for your application or is simply Application.


2. Your main method could be split into multiple methods, at least including
   
   
   • creating Authentication.uniqueStudent from std.bat,
   
   
   • handling each case and
   
   
   • handling objective.getTotalMoney() >= 800.
   
   


3. 
   std.bat is an unfortunate name for a serialised object, because .bat is usually associated with Batch script files. I don't know what the standard is for this, but I imagine something like .object, .jo (for Java object) or .Student would work.


4. The code only ever reads and writes a single student to the same file. A production application would support many students, and would use a more portable system like a database to persist information.


5. Naming things according to what they contain or do rather than what they are is very useful for comprehension. For example, myfine (sic) could be something like studentFile and sc could be inputScanner. Short variable names are a thing of the past, before code completion became ubiquitous.


6. It would be difficult to automate command line interaction with your system since it is interactive. I would instead make it accept parameters to decide what to do and which information to do it with, like almost every shell tool (find, grep, wc, etc.). Interactivity can then be tacked on if at all necessary.


7. 
   800 is a "magic" amount of money in two places in your code. It should ideally be configurable since it clearly would change over time in the real world.


8. Being able to pass an existing student ID would be extremely useful if this code is to be used with an existing system.


9. Four random characters chosen from 31 does make for a lot of possible IDs, but why not just use a serial number and persisting that information?


10. Using abcdefghijklmnopqrstuvwxyz12345 as the possible characters for an ID is a bit strange. Why not six through nine or zero? Why not upper case characters? Why not the entire base 64 range, all printable characters or even all of Unicode?


11. I can't tell at a glance whether (int) (Math.random() * pick.length()) manages to land on each index uniformly. These solutions are more obviously intuitively correct.


12. Having a getter and setter for each field is considered an antipattern by many developers. I don't have an authoritative reference, but I would encourage reading up on the arguments given.


13. In general methods returning null should only happen in case of a bug, because it complicates handling the response. So a method which returns ArrayList<Foo> should return an empty list if there are no results, not null. You can also look into optionals for Java 8 and newer.


14. You have unused code, such as String unique = "" + getGrade();, where the value is thrown away two lines below, and Student.main.


15. 
    Authentication has nothing at all to do with authentication.

In conclusion, this code is mostly fine for homework, but I'm afraid it's far from production ready.