bad code to good one – Java – Part 1

Well, here is an interesting case. I found this code in one of my old machines.

The code is waiting for input from the user and fills a hashmap. Yet, while extracting, simply it throws an exception. Our first question is how to fix it, then second is how to really improve it?

import java.util.HashMap;
import java.util.Iterator;
import java.util.Scanner;

class Person {
 protected String name;
 protected char gender;
 protected int age;

 public Person(String name, char gender, int age) {
 this.name = name;
 this.gender = gender;
 this.age = age;
 }

 public String toString() {
 return ("\nName: " + name + "\nGender: " + gender + "\nAge: " + age);
 }
}

class Student extends Person {
 protected String sid;
 protected double gpa;
 protected Course c;

 public Student(String name, char gender, int age, String sid, double gpa, Course c) {
 super(name, gender, age);
 this.sid = sid;
 this.gpa = gpa;
 this.c = c;
 }

 public String toString() {
 return (super.toString() + "\nStudent id: " + sid + "\nGpa: " + gpa);
 }
}

class Course {
 protected String cid;
 protected String courseName;
 public Course(String cid, String courseName) {
 this.cid = cid;
 this.courseName = courseName;
 }

 public String toString() {
 return ("\nCourse Id: " + cid + "\nCourse name: " + courseName + "\n");
 }
}

public class StudentCourse {
 public static void main(String[] args) {
 Scanner scanner = null;
 String courseName = null;
 String courseId = null;

 HashMap < Integer, String > hm = new HashMap < Integer, String > ();
 int i = 0;
 while (i < 2) {
 System.out.println("Enter course id:");

 scanner = new Scanner(System.in);

 courseId = scanner.next();

 System.out.println("Enter course name:");

 courseName = scanner.next();

 /*
 if(courseId == "-1" && courseName.equals("F"))
 break;*/
 hm.put(Integer.valueOf(courseId), courseName);
 i++;
 }

 System.out.println("Name: ");
 String name = scanner.next();

 System.out.println("Gender: ");
 String gender = scanner.next();

 System.out.println("Age: ");
 int age = scanner.nextInt();

 System.out.println("Student Id: ");
 String studentId = scanner.next();

 System.out.println("Student Gpa: ");
 double gpa = scanner.nextDouble();


 if (hm.size() == 1) {
 Course course = new Course(courseId, courseName);
 Student student = new Student(name, gender.charAt(0), age, studentId, gpa, course);

 System.out.println(student);
 System.out.println(course);
 } else {
 Iterator it1 = hm.keySet().iterator();
 Iterator it2 = hm.entrySet().iterator();
 while (it1.hasNext()) {
 int key = (Integer) it1.next();
 String value = (String) it2.next();

 Course course = new Course(String.valueOf(key), value);
 Student student = new Student(name, gender.charAt(0), age, studentId, gpa, course);
 System.out.println(student);
 System.out.println(course);
 }

 }
 }
}

Above code will throw an exception. So, I changed

String value = (String)it2.next(); to this     String value = hm.get(key);

The refactored code is as follows which will work “fine”:

import java.util.HashMap;
import java.util.Iterator;
import java.util.Scanner;

class Person {
 protected String name;
 protected char gender;
 protected int age;

 public Person(String name, char gender, int age) {
 this.name = name;
 this.gender = gender;
 this.age = age;
 }

 public String toString() {
 return ("\nName: " + name + "\nGender: " + gender + "\nAge: " + age);
 }
}

class Student extends Person {
 protected String sid;
 protected double gpa;
 protected Course c;

 public Student(String name, char gender, int age, String sid, double gpa, Course c) {
 super(name, gender, age);
 this.sid = sid;
 this.gpa = gpa;
 this.c = c;
 }

 public String toString() {
 return (super.toString() + "\nStudent id: " + sid + "\nGpa: " + gpa);
 }
}

class Course {
 protected String cid;
 protected String courseName;
 public Course(String cid, String courseName) {
 this.cid = cid;
 this.courseName = courseName;
 }

 public String toString() {
 return ("\nCourse Id: " + cid + "\nCourse name: " + courseName + "\n");
 }
}

public class StudentCourse {
 public static void main(String[] args) {
 Scanner scanner = null;
 String courseName = null;
 String courseId = null;

 HashMap < Integer, String > hm = new HashMap < Integer, String > ();
 int i = 0;
 while (i < 2) {
 System.out.println("Enter course id:");

 scanner = new Scanner(System.in);

 courseId = scanner.next();

 System.out.println("Enter course name:");

 courseName = scanner.next();

 /*
 if(courseId == "-1" && courseName.equals("F"))
 break;*/
 hm.put(Integer.valueOf(courseId), courseName);
 i++;
 }

 System.out.println("Name: ");
 String name = scanner.next();

 System.out.println("Gender: ");
 String gender = scanner.next();

 System.out.println("Age: ");
 int age = scanner.nextInt();

 System.out.println("Student Id: ");
 String studentId = scanner.next();

 System.out.println("Student Gpa: ");
 double gpa = scanner.nextDouble();


 if (hm.size() == 1) {
 Course course = new Course(courseId, courseName);
 Student student = new Student(name, gender.charAt(0), age, studentId, gpa, course);

 System.out.println(student);
 System.out.println(course);
 } else {
 Iterator it1 = hm.keySet().iterator();
 Iterator it2 = hm.entrySet().iterator();
 while (it1.hasNext()) {
 int key = (Integer) it1.next();
 String value = hm.get(key);

 Course course = new Course(String.valueOf(key), value);
 Student student = new Student(name, gender.charAt(0), age, studentId, gpa, course);
 System.out.println(student);
 System.out.println(course);
 }

 }
 }
}

However, above code is still not in its prime. It should be refactored further:

        for( Map.Entry<Integer, String> entry : hm )
                  {
                        int key = entry.getKey();
                        String value = entry.getValue();
                        
                        Course course = new Course(String.valueOf(key), value);
                        Student student = new Student(name, gender.charAt(0), age, studentId ,gpa, course);
                        System.out.println(student);
                        System.out.println(course);
                  }