Java - How to organize Map with a List as values

Question

I have been given a problem where I have to organize a map with the values being lists. I am given a pre-written class called Student. I have to write a method that returns a map such as this:

Map<String, List<Student>> map;

The keys are supposed to represent the classes in which the students are enrolled in and the list is supposed to represent the students in that class. I am given a list of students and have to organize those students into the map by order of lowest to highest GPA.

This is the Student class that I am given:

public static class Student {
    private String name;
    private int zip;
    private Map<String, Double> grades;

    public Student(String name, int zip) {
        this.name = name;
        this.zip = zip;
        grades = new HashMap<String, Double>();
    }

    public void addGrade(String n, double d) {
        grades.put(n, d);
    }

    public double getGrade(String n) {
        return grades.get(n);
    }

    public Map<String, Double> getGrades(){
        return grades;
    }

    public double getOverAllGrade() {
        //To do as part of the test
        double totalGrade = 0.0;
        for (String grade : grades.keySet()) {
            totalGrade += getGrade(grade);
        }
        double average = totalGrade / grades.size();
        return average;
    }

    public int getZip() {
        return zip;
    }

    public String getName() {
        return name;
    }

    public boolean equals(Object o) {
        Student s = (Student)o;
        return zip == s.zip && name.equals(s.name);
    }

    public String toString() {
        return name+" "+zip;
    }

}

I have already attempted to solve this problem with the following code (I have included the purpose of the method to help clear up an confusion):

/**
 * return a Map<String, List<Student>> of all the classes mapped
 * to a List of Students in the order of their grades from lowest
 * to highest
 * 
 * @param students List<Student> the students in the school
 * @return Map<String, List<Student>> {"class name":{Student1, Student2}, "class 2":{Student3, Student4"}} 
 */
public static Map<String, List<Student>> organizeByClass(List<Student> students){
    HashMap<String, List<Student>> map = new HashMap<String, List<Student>>();
    for (Student student : students) {
        for (String className : student.getGrades().keySet()) {
            if (!map.keySet().contains(className)) {                                           // If the class hasn't been added, it will be added
                ArrayList<Student> studentsInClass = new ArrayList<Student>();
                studentsInClass.add(student);
                map.put(className, studentsInClass);
            } else {
                if (student.getOverAllGrade() < map.get(className).get(0).getOverAllGrade()) { // Sorts the students by GPA
                    map.get(className).add(0, student);
                } else {
                    map.get(className).add(student);
                }
            }
        }
    }
    return map;
}

This is the test case that I am using (It is using the JUnit library):

@Test
public void organizeByClassTest1() {
    List<Student> col = new ArrayList<Student>();
    col.add(createStudent("Bob", 12345));
    col.add(createStudent("Earl", 67890));
    col.add(createStudent("Roman", 12345));
    col.add(createStudent("Victor", 45678));
    col.add(createStudent("Nick", 67890));
    col.add(createStudent("Aaron", 12345));
    col.add(createStudent("Marissa", 45678));
    col.add(createStudent("Enoch", 12345));
    col.add(createStudent("Spencer", 12345));
    col.add(createStudent("Britt", 45678));
    Student s1 = createStudent("Justin", 67890);
    col.add(s1);
    Map<String, List<Student>> map1 = new HashMap<String, List<Student>>();
    Random r = new Random();
    List<Student> g1 = new ArrayList<Student>();
    List<Student> h1 = new ArrayList<Student>();
    List<Student> he1 = new ArrayList<Student>();
    List<Student> cs1 = new ArrayList<Student>();
    List<Student> p1 = new ArrayList<Student>();

    map1.put("Geometry", g1);
    map1.put("History", h1);
    map1.put("Home Economics", he1);
    map1.put("Physics", p1);
    map1.put("Computer Science", cs1);
    for (Student s : col) {
        if (r.nextInt(5) > 0) {
            s.addGrade("Geometry", r.nextDouble() * 4);
            g1.add(s);
        }
        if (r.nextInt(5) > 0) {

            s.addGrade("Computer Science", r.nextDouble() * 4);
            cs1.add(s);
        }
        if (r.nextInt(5) > 0) {
            s.addGrade("History", r.nextDouble() * 4);
            h1.add(s);
        }
        if (r.nextInt(5) > 0) {
            s.addGrade("Home Economics", r.nextDouble() * 4);
            he1.add(s);
        }
        if (r.nextInt(5) > 0) {
            s.addGrade("Physics", r.nextDouble() * 4);
            p1.add(s);
        }
    }
    Comparator<Student> c = new Comparator<Student>() {
        public int compare(Student s1, Student s2) {
            return (int) (s1.getOverAllGrade()-s2.getOverAllGrade());
        }
    };
    Collections.sort(p1, c );
    Collections.sort(he1, c );
    Collections.sort(h1, c );
    Collections.sort(cs1, c );
    Collections.sort(g1, c );
    Map<String,List<Student>> result = MappingAssessment.organizeByClass(col);
    assertEquals(map1,result);
}

I do not understand why the code I have written is incorrect. All help is appreciated.

Thanks, Steward.

Answer 1

You should also be using a LinkedList<Student> instead of an ArrayList<Student> since you do so many insertions every time you encounter a new student. Each time the list of students exist in the map already (your else {...} block), you need to loop through and sort the list rather than add to the back of the list.

Answer 2

You aren't sorting the students properly as any student who isn't the highest grade is just sent to the back of the list.