HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

School grading string calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gradingcalculatorstringschool

Problem

For my training, i've been given an assignment to build a simple, extendable 'grade' String calculator by a given grade.

For now, I had to work by this table:

When the grade was higher then 100, I had to return the String "You must have been cheating".

What I currently came up with is:

public String gradeBy(int grade) {
    TreeMap grades = new TreeMap<>();
    grades.put(grade = 51 && grade = 61 && grade = 71 && grade  80 && grade  currentGrade : grades.entrySet()) {
        if (currentGrade.getKey() != true) {
            continue;
        }
        return currentGrade.getValue().name();
    }
    return "You must have been cheating";
}

enum Grade {
    F,
    D,
    C,
    B,
    A
}


Where I test it as:

@Test
public void gradeBy() throws Exception {
    int[] grades = {45, 52, 66, 93, 102};
    for(int grade : grades) {
        String gradeString = this.grading.gradeBy(grade);
        System.out.println(grade + "gives :"+ gradeString);
    }
}


Can this be improved in a more OOP way? Can the grading list be apart from the grading system it self?

Solution

What stands out to me is that you initially perform a check against every grade range and insert true into the map in the right one. By that point, you've already found which grade is right. But then you initiate another loop to start searching for the grade corresponding to true.

I think instead, you ought to alter your method to output a Grade enum, and delegate the string handling to another function. I feel that it's better practice to return a Grade Enum from the first method because of the strong typing-- you may want to use this information for other methods besides finding the grade name. In fact, Enums are particularly nice because you can use a switch on them.

public Grade gradeBy(int grade) {
    if (grade < 50) return Grade.F;
    if (grade < 61) return Grade.D;
    if (grade < 71) return Grade.C;
    if (grade < 81) return Grade.B;
    if (grade<=100) return Grade.A;
    return Grade.F;
}
public String gradeName(Grade g) {
    return g.name()
}


Notice how as soon as the grade matches the range corresponding to something in the Grade Enum, the method immediately returns. Now an example of how you might use that switch:

public String gradeComment(Grade g) {
    switch(g) {
        case F:
        return "You need to work harder!";
        case D:
        return "I think you can do better.";
        case C:
        return "Satisfactory.";
        case B:
        return "Nice job.";
        case A:
        return "Well done!";
        default:
        return "N/A";
    }
}


Not that you need these grade messages, but that's an example of the power of holding on to the Enum type rather than converting the int grade directly into a String.

Sorry if I've made syntax errors as I've not used Java in a while.

Edit: Just realised that you might like to put gradeBy, gradeName and gradeComment into the Grade Enum itself as static methods.

Code Snippets

public Grade gradeBy(int grade) {
    if (grade < 50) return Grade.F;
    if (grade < 61) return Grade.D;
    if (grade < 71) return Grade.C;
    if (grade < 81) return Grade.B;
    if (grade<=100) return Grade.A;
    return Grade.F;
}
public String gradeName(Grade g) {
    return g.name()
}
public String gradeComment(Grade g) {
    switch(g) {
        case F:
        return "You need to work harder!";
        case D:
        return "I think you can do better.";
        case C:
        return "Satisfactory.";
        case B:
        return "Nice job.";
        case A:
        return "Well done!";
        default:
        return "N/A";
    }
}

Context

StackExchange Code Review Q#142708, answer score: 5

Revisions (0)

No revisions yet.