The method getGpa is still a bit long and difficult to follow. Extract from it a new method that returns a double value for a given letter grade: double getGpa() { if (grades.isEmpty()) return 0.0; double total = 0.0; for (String grade: grades) total += gradePointsFor(grade); return total / grades.size(); } int gradePointsFor(String grade) { if (grade.equals("A")) return 4; else if (grade.equals("B")) return 3; else if (grade.equals("C")) return 2; else if (grade.equals("D")) return 1; return 0; } Make sure this compiles and passes the tests. One further change that you can make is to remove the else clauses. A return statement is a flow of control. Once the Java VM encounters a return statement in a method, it executes no more statements within the method. int gradePointsFor(String grade) { if (grade.equals("A")) return 4; if (grade.equals("B")) return 3; if (grade.equals("C")) return 2; if (grade.equals("D")) return 1; return 0; } In the gradePointsFor method, as soon as there is a match on grade, the VM executes a return statement and makes no more comparisons. You'll also note that I put each return on the same line as the corresponding if statement conditional. Normally, you will want to keep the conditional and the body of an if statement on separate lines. In this example, the multiple if statements are repetitive, terse, and visually consistent. The code is easy to read and digest. In other circumstances, jumbling the conditional and body together can result in an unreadable mess. You can refactor the test itself: public void testCalculateGpa() { Student student = new Student("a"); assertGpa(student, 0.0); student.addGrade("A"); assertGpa(student, 4.0); student.addGrade("B"); assertGpa(student, 3.5); student.addGrade("C"); assertGpa(student, 3.0); student.addGrade("D"); assertGpa(student, 2.5); student.addGrade("F"); assertGpa(student, 2.0) ; } private void assertGpa(Student student, double expectedGpa) { assertEquals(expectedGpa, student.getGpa(), GRADE_TOLERANCE); } |