CODEWITHSUNDEEP
javaenumsswitch-statementcase
Thaitan
Thaitan

Reputation: 35

Switch statement returning null

I'm trying to implement a grading system for a test using an enum and a switch statement, however with the current code I have the result is always "null". I can't see where iv'e gone wrong, can anyone help? 

public enum Grade {
    A, B, C, D, E, U; 
}
public static void main(String[] args) {
    Scanner scan = new Scanner(System.in);
    System.out.println("Enter the students mark:");
    int points = scan.nextInt();

    if (points < 0 || points > 200) {
        System.out.println("Error! points must be between 0 & 200");
    } else {
       System.out.println(findGrade(points));
    }

}

public static Grade findGrade(int points) {
    switch (points) {
        case 1:
            if (points>= 0 && points <= 59) {
                return Grade.valueOf("U");
            }
        case 2:
            if (points >= 60 && points <= 89) {
                return Grade.valueOf("E");
            }
        case 3:
            if (points >= 90 && points <= 119) {
                return Grade.valueOf("D");
            }
        case 4:
            if (points >= 110 && points <= 139) {
                return Grade.valueOf("C");
            }
        case 5:
            if (points >= 140 && points <= 169) {
                return Grade.valueOf("B");
            }
            case 6:
            if (points >= 170 && points <= 200) {
                return Grade.valueOf("A");
            }
        default:
            return null;
    }
}

}

Upvotes: 2

Views: 4510

Answers (3)

Zong
Zong

Reputation: 6230

Let's look at 

switch (points) {
   case 1:
      if (points >= 0 && points <= 59) {
          return Grade.valueOf("U");
      }

What you're basically saying is: 

if (points == 1) {
    if (points >= 0 && points <= 59) {
        return Grade.valueOf("U");
    }
}

which is nonsense. I don't think you need switch at all in this case. Just use: 

if (points < 0) {
    return null;
}
if (points <= 59) {
    return Grade.valueOf("U");
}
if (points <= 89) {
    return Grade.valueOf("E");
}
if (points <= 119)
    return Grade.valueOf("D");
}
...
return null;

Upvotes: 5

Rohit Jain
Rohit Jain

Reputation: 213281

While switching on an int type variable say points, the code corresponding to a case n is executed, when the value of points is n. I guess you can now understand where the mistake is. Currently, none of your cases would match the value of points, if it is greater than 6, so the value will be null.

It is clear that you want to implement cases such that those are executed for various ranges. One way to implement this is by narrowing a larger range to smaller range, so that writing cases for them is easier for you. For example, the range [0, 59] can be narrowed to [0, 5] by dividing points by 10. Similarly for other ranges, you can narrow them, and write cases like these:

public static Grade findGrade(int points) {
    int val = points / 10;

    switch (val) {
        // 0 <= points <= 59 is same as 0 <= val <= 5
        case 0: case 1: 
        case 2: case 3: 
        case 4: case 5: return Grade.valueOf("U");

        // 60 <= points <= 89 is same as 6 <= val <= 8
        case 6: case 7: 
        case 8:         return Grade.valueOf("E");

        // like so
    }
}

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726639

You do not need a switch at all. Simply put your ifs in a chain, and that would work for you:

if (points>= 0 && points <= 59) {
    return Grade.valueOf("U");
}
if (points >= 60 && points <= 89) {
    return Grade.valueOf("E");
}
if (points >= 90 && points <= 119) {
    return Grade.valueOf("D");
}
if (points >= 110 && points <= 139) {
    return Grade.valueOf("C");
}
if (points >= 140 && points <= 169) {
    return Grade.valueOf("B");
}
if (points >= 170 && points <= 200) {
    return Grade.valueOf("A");
}
return null;

The idea behind the switch statement is to let you base a decision on a set of fixed values or small value ranges. In this case, however, the range is pretty large, so a chain of if statements looks like a more appropriate choice.

Considering the task that you are implementing, you could build a shorter solution with a lookup table and a loop:

int[] limits = new int[] {59, 89, 119, 139, 169, 200};
String[] grades = new String[] {"U", "E", "D", "C", "B", "A"};
for (int i = 0 ; i != limits.length ; i++)
    if (points <= limits[i])
         return grades[i];
return null;

Upvotes: 2

Related Questions