5
\$\begingroup\$

I have bunch of similar methods in different classes to average student grades.

In the Student class:

public double getAverageGrade() throws NoGradesForStudentException {
    double sum = 0;
    int totalGrades = 0;
    ArrayList<Subject> studentSubjects = getStudentSubjects();
    for(Subject subject : studentSubjects) {
        try {
            sum += getAverageGradeInSubject(subject);
            totalGrades++;
        }
        catch(NoGradesInSubjectException e) {
            //everything is okey
        }
    }
    if (totalGrades == 0) {
        throw new NoGradesForStudentException("Student does not have any grades yet");
    } else {
        return sum/totalGrades;
    }
}

In the GroupOfCourses class:

public double getAverageGrade() throws NoGradesInCourseException {
    double gradeSum = 0;
    double studentsWithGrades = 0;
    for(Student student : getStudents()) {
        try {
            gradeSum += student.getAverageGrade();
            studentsWithGrades++;
        }
        catch (NoGradesForStudentException e) {
            //viss kartiba, ja izmet so iznemumu
        }
    }
    if (studentsWithGrades != 0) {
        return gradeSum/studentsWithGrades;
    } else {
        throw new NoGradesInCourseException("There are no grades in course.");
    }
}

This one is in the main and user input is mingled with the logic. But the logic is the same as in previous examples.

private static void calculateSubjectAvgGrade() {
    ArrayList<Subject> allSubjects = Main.getSubjectsInAllCourses();

    for(int index = 0; index < allSubjects.size(); index++) {
        System.out.println(index + ". " + allSubjects.get(index).getTitle());
    }
    System.out.print("Izvelieties prieksmetu ievodot ta numuru: ");
    int number = Input.consoleIntIn();

    double sum = 0;
    int subjectsWithGrade = 0;
    for (Course course : Main.courses) {
        try {
            sum += course.getAverageGradeInSubject(allSubjects.get(number));
            subjectsWithGrade++;
        }
        catch(NoGradesInSubjectException e) {
            //viss ok
        }
    }
    double avg = subjectsWithGrade != 0 ? sum/subjectsWithGrade : 0;
    System.out.println("Prieksmeta videja atzime ir: " + avg);
}

I was wondering is there a way I could create one function and call it in those methods? It would look something like this:

public double getAvarageGrade() throws NoGradesException {
     return function(studentSubjects, "Student does not have any grades yet");
}

I'm asking this because I have lot of methods in different classes that duplicate their logic.

I'm using Java 8.

\$\endgroup\$
5
\$\begingroup\$

Since you are writing for Java 8, what you want to use is DoubleStream.average(). Java 8 streams are a new and powerful technique that drastically changes how code can be written in Java.

public class Student {

    ...

    public double getAverageGrade() throws NoGradesForStudentException {
        return getStudentSubjects().stream()
                                   .mapToDouble(Subject::getAverageGradeInSubject)
                                   .average()
                                   .orElseThrow(() -> new NoGradesForStudentException("Student does not have any grades yet"));
    }
}

This code is compact enough that near-duplication of code would not be a problem.

However, the NoGradesForStudentException would cause a bit of a problem. In particular, with a stream-based solution, ignoring cases where a student has no grade in a particular subject would be a tricky exercise in exception handling. A better approach is to embrace Java's convention for DoubleStream.average(), and change all of these getAverageGrade…() methods to return an OptionalDouble instead of a double. Then, the code would look like this:

public class Student {

    ...

    public OptionalDouble getAverageGrade() {
        return getStudentSubjects().stream()
                                   .mapToObj(Subject::getAverageGradeInSubject)
                                   .filter(OptionalDouble::isPresent)
                                   .mapToDouble(OptionalDouble::getAsDouble)
                                   .average();
    }
}

Note that taking the average of the average of grades in each subject is not necessarily the same as taking the average of grades. It may be appropriate to rename the method to make it clear that there is batching by subject. Similarly, your GroupOfCourses.getAverageGrade() needs to make it clear that it calculates the average of each student's grade.

\$\endgroup\$
2
\$\begingroup\$

It's not the prettiest code, but a generic average function might look like this:

   public interface Input {
        int getInt(Object input);
    }

    public static double average(List<?> list, Input input) {
        if (list.isEmpty()) {
            throw new IllegalArgumentException();
        }

        double sum = 0;
        int totalGrades = 0;
        for (Object item : list) {
            try {
                sum += input.getInt(item);
                totalGrades++;
            } catch (Exception e) {
                //everything is okey (is it though?)
            }
        }
        return sum / totalGrades;
    }

    public double getAverageGrade() throws NoGradesForStudentException {
        List<Subject> studentSubjects = getStudentSubjects();
        return average(studentSubjects, (subject) -> getAverageGradeInSubject((Subject) subject));
    }

    public double getAverageGrade() throws NoGradesInCourseException {
        List<Student> students = getStudents();
        return average(students, (student) -> ((Student) student).getAverageGrade());
    }

As to your code:

  • why do you swallow the NoGradesInSubjectException exception? It doesn't seem like a good idea.
  • try to be consistent. In one method you use totalGrades == 0, but in another you turn the check around with studentsWithGrades != 0. I actually prefer my way of checking for an empty list at the very beginning.
\$\endgroup\$
  • \$\begingroup\$ I catch and throw those exceptions because if student does not have any grades it would not return 0 as it could be students average grade. When exception is caught here ` //everything is okey (is it though?)` then nothing is added to the sum and count of grades is not increased. \$\endgroup\$ – user255572 Mar 27 '15 at 23:28

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.