5
\$\begingroup\$

Here is a class I made that is able to ask a teacher what percentage their student has in a class and gives them back a grade. The code is very repetitive and I am wondering what I can do to simplify the class below before inputting any more grades.

import java.util.Scanner;

public class Grades{
    public static void main(String [] args){
        Scanner grades = new Scanner(System.in);

        int x = 0;

        System.out.print("What is the percentage of the student: ");
        x = grades.nextInt();

        if (x > 100 || x < 0){
                System.out.println("Please put in a percent from 0 - 100!\n");
        }

        while (x > 100 || x < 0 ){

            System.out.print("What is the percentage of the student: ");
            x = grades.nextInt();
            if (x > 100 || x < 0){
                System.out.println("Please put in a percent from 0 - 100!\n");
            }
        }
        if (x <= 100 && x >= 96){
             System.out.print("A+");
        }
         if (x <= 95 && x >= 92){
             System.out.print("A");
        }
         if (x <= 91 && x >= 90){
             System.out.print("A-");
        }
         if (x <= 89 && x >= 50){
             System.out.print("B+");
        }
         if (x <= 49 && x >= 0){
             System.out.print("B");
        }
        // so on and so forth down the grading scale
    }
}
\$\endgroup\$
5
\$\begingroup\$

It would be good to split the different sub-tasks to different functions, for example:

  • int promptScore(): a method to prompt for score
  • String getGrade(int score): a method to return the grade from the score

The promptScore method could use an infinite loop with two steps:

  1. Ask for a number
  2. If the number is within the range, return it

The getGrade method could use a sorted map that stores the limits and the corresponding grades. You could iterate over the entries in this map, and when you find a value that is below the limit, then you return the corresponding grade.

Something like this:

class Grades {

    private static final Map<Integer, String> limits;

    static {
        limits = new TreeMap<>();
        limits.put(50, "B");
        limits.put(90, "B+");
        limits.put(92, "A-");
        limits.put(96, "A");
        limits.put(101, "A+");
    }

    public static int promptScore() {
        Scanner scanner = new Scanner(System.in);

        while (true) {
            System.out.print("What is the percentage of the student: ");
            int x = scanner.nextInt();

            if (0 <= x && x <= 100) {
                return x;
            }
            System.out.println("Please put in a percent from 0 - 100!\n");
        }
    }

    public static String getGrade(int score) {
        for (Map.Entry<Integer, String> entry : limits.entrySet()) {
            int limit = entry.getKey();
            if (score < limit) {
                return entry.getValue();
            }
        }
        throw new AssertionError("Impossible case");
    }

    public static void main(String[] args) {
        System.out.println(getGrade(promptScore()));
    }
}

The benefit of splitting up the functions like this is that writing unit tests becomes easy:

@Test
public void testB() {
    assertEquals("B", Grades.getGrade(0));
    assertEquals("B", Grades.getGrade(49));
}

@Test
public void testB_Plus() {
    assertEquals("B+", Grades.getGrade(50));
    assertEquals("B+", Grades.getGrade(89));
}

@Test
public void testA_Minus() {
    assertEquals("A-", Grades.getGrade(90));
    assertEquals("A-", Grades.getGrade(91));
}

@Test
public void testA() {
    assertEquals("A", Grades.getGrade(92));
    assertEquals("A", Grades.getGrade(95));
}

@Test
public void testA_Plus() {
    assertEquals("A+", Grades.getGrade(96));
    assertEquals("A+", Grades.getGrade(100));
}
\$\endgroup\$
5
\$\begingroup\$

Your variable naming is a bit off, I think. x would be better named score or percentage. grades would probably be better as just input.

The validation loop could be structured more smartly.

As for handling all those cases, I suggest using an enum that defines all of the threshold scores.

import java.util.Scanner;

public enum Grade {
    A_PLUS (95, "A+"),
    A      (92, "A"),
    A_MINUS(90, "A-"),
    B_PLUS (50, "B+"),
    B      ( 0, "B");

    public static final int MINIMUM = 0, MAXIMUM = 100;

    public final int threshold;
    public final String letter;
    Grade(int threshold, String letter) {
        this.threshold = threshold;
        this.letter = letter;
    }

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        int score;

        do {
            System.out.print("What is the percentage of the student: ");
            score = input.nextInt();
            if (MINIMUM <= score && score <= MAXIMUM) {
                break;
            }
            System.out.println("Please put in a percent from 0 to 100!");
        } while (true);

        for (Grade grade : values()) {
            if (score >= grade.threshold) {
                System.out.println(grade.letter);
                break;
            }
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$
  • You can remove the first check if x is inside the allowed range if you put the following output at the top of the while loop.
    This has also the advantage of removing the if statement
  • the first condition at checking the grades can be simplified to (x >= 96)
  • you should always name your variables meaninful. So x could be renamed to percentage.
  • you should extract the "calculation" of the grade to a separate method, which takes the x as input parameter and returns either an enum, which you can pass to a "print" method and based on a switch statement produces the output. Or you can return from the "calculation" method the index of an array containing the grades as string which then can be used for the print method.
\$\endgroup\$
2
\$\begingroup\$

I think you should try to separate better then concerns of your application, creating different functions for the different logical tasks you do.

I'd create a function to handle/validate interaction with the user and another function to convert your numeric value to the corresponding grade.

You're right that your approach has lots of duplication. You can easily improve it by creating a sorted map or a list of pairs of marks and their lower bounds.

In your case the list should contain:

 0 B
50 B+
90 A-
92 A
96 A+

you can easily determine what is the corresponding mark by going through the list until you find a lower bound greater than the value you have. For instance, if your input value is 91 you first evaluate (0,B). 91 is bigger than 0 so you can move on to (50,B+). Ditto for (90,A-). Finally you get to (92,A), you see that 91 < 92 and hence you know you have to return A-.

\$\endgroup\$
1
\$\begingroup\$

This should simplify things some, its just a re-arrangement of the logic. There were also a few places that should have used println instead of print I think.

import java.util.Scanner;

public class Grades {
    public static void main(String [] args) {
        Scanner grades = new Scanner(System.in);
        int x = 0;

        while(true) {
            System.out.println("What is the percentage of the student: ");
            x = grades.nextInt();

            if (x >= 0 && x <= 100)
                break;

            System.out.println("Please put in a percent from 0 - 100!\n");
        }

        if (x <= 100 && x >= 96) {
             System.out.println("A+");
        }
        else if (x <= 95 && x >= 92) {
             System.out.println("A");
        }
        else if (x <= 91 && x >= 90) {
             System.out.println("A-");
        }
        else if (x <= 89 && x >= 50) {
             System.out.println("B+");
        }
        else if (x <= 49 && x >= 0) {
             System.out.println("B");
        }
        // so on and so forth down the grading scale
    }
}
\$\endgroup\$
0
\$\begingroup\$

You could put the stuff in a table:

class grade_table
{
  int range_low;
  int range_high;
  char letter_grade;
};

All you would need to do is to search the table for the range that encompasses the grade and the pull out the letter grade. This may be simpler than all those if-else-if statements.

\$\endgroup\$

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.