case switch java compile error

Question

I wrote the following code but get the following compile error:

The local variable dirArrow may not have been initialized. Note that a problem regarding missing 'default:' on 'switch' has been suppressed, which is perhaps related to this problem

//return the ID of the robot and arrow of his facing direction
public String toString(){
    char dirArrow;
    switch (direction) {
        case UP: dirArrow= '^';
        case RIGHT: dirArrow= '>';
        case DOWN: dirArrow= 'V';
        case LEFT: dirArrow= '<';
        break;

    }
    return (Integer.toString(RoboID) + dirArrow);
}

add a `default` case after your last case, as the error message says. Also you should use `break` after every `case` or else it will fall-through. — QBrute, Apr 29 '14 at 11:23
default in switch is not strictly necessary, in your case I would probably just omit myself. — kviiri, Apr 29 '14 at 11:27
4 answers which *all* recommend initializing a `char` variable with a `null` value? Yikes! — Jon Skeet, Apr 29 '14 at 11:29
And now most of the answers have changed to try using `''` which isn't a valid character literal either. Yikes. — Jon Skeet, Apr 29 '14 at 11:37

score 2 · Answer 1 · edited May 23 '17 at 11:49

2

You need to initialize your dirArrow variable like:

char dirArrow = ' ';
switch (direction) {

Read why local variable should be initialized.

Note: also you need to add a break statement to the end of each case block like:

case UP: {
    dirArrow= '^';
    break;
}

edited May 23 '17 at 11:49

Community

1
1

answered Apr 29 '14 at 11:24

Salah

8,567
3
26
43

1

Also mention that `break;` should be added after *each* `case`. – Maroun Apr 29 '14 at 11:25
2

`null` isn't a valid `char`. – Jon Skeet Apr 29 '14 at 11:28
This still isn't the cleanest approach, IMO. – Jon Skeet Apr 29 '14 at 11:39

score 1 · Answer 2 · edited Apr 29 '14 at 11:38

1

You have two problems there

You haven't initialize the method local variable dirArrow, initialize it like char dirArrow = ' ';
You don't have break at the end of each case. If you don't add break at the end of each case, for the first case, it will run all the case statements below that.

switch (direction) {
        case UP: dirArrow= '^'; break;
        case RIGHT: dirArrow= '>'; break;
        case DOWN: dirArrow= 'V'; break;
        case LEFT: dirArrow= '<'; break;

}

edited Apr 29 '14 at 11:38

Hello World

944
2
15
39

answered Apr 29 '14 at 11:24

Abimaran Kugathasan

31,165
11
75
105

How do you expect `char dirArrow = null;` to compile? – Jon Skeet Apr 29 '14 at 11:29
1

char dirArrow = ''; doesnt work also, only with space between the ' ' – Yuval Levy Apr 29 '14 at 11:35
@YuvalLevy : Yes, there should be a space, like `' '` – Abimaran Kugathasan Apr 29 '14 at 11:37

SpringLearner · Answer 3 · 2014-04-29T11:29:58.317

0

its very much clear from the error that you have to initialize the local variable like '' like

char dirArrow='';

edited Apr 29 '14 at 11:29

answered Apr 29 '14 at 11:24

SpringLearner

13,738
20
78
116

if I am trying char dirArrow=null; i get: "Type mismatch: cannot convert from null to char" error – Yuval Levy Apr 29 '14 at 11:28
1

`null` is not a valid value for `char`. – Jon Skeet Apr 29 '14 at 11:28
@JonSkeet Thanks for poiting out the mistake – SpringLearner Apr 29 '14 at 11:31
`''` isn't a valid character literal either... and this isn't the cleanest fix anyway. – Jon Skeet Apr 29 '14 at 11:37

score 0 · Answer 4 · answered Apr 29 '14 at 11:28

either initialise the variable at declaration: char dirArrow = 0; or add a default block:

switch (direction) {
    case UP: dirArrow= '^';
    case RIGHT: dirArrow= '>';
    case DOWN: dirArrow= 'V';
    case LEFT: dirArrow= '<';
    break;
    defalut: dirArrow = 0;
}

BTW: for your code to work as expected, you should add a break after each case statement:

switch (direction) {
    case UP: dirArrow= '^';    break;
    case RIGHT: dirArrow= '>'; break;
    case DOWN: dirArrow= 'V';  break;
    case LEFT: dirArrow= '<';  break;

    defalut: dirArrow = 0;
}

Jon Skeet · Answer 5 · 2014-04-29T11:42:05.413

You haven't said what you want to happen if the direction isn't any of those values. Maybe that's currently the full set of directions, but in the future it may not be - and fundamentally the Java compiler doesn't check that you've covered every enum possibility. (You're also missing break statements, as noted by other answerers.)

Personally I'd change the approach and get rid of the switch entirely: add a toChar method to Direction, and then you can just use:

return Integer.toString(RoboID) + direction.toChar();

Your enum constructor would take the character to use as well, so it would be very easy to implement.

Much cleaner, IMO. The only downside is that it ties your character representation to the Direction enum which may be intended to be UI-neutral. If that's the case, I'd have a Map<Direction, Character> instead, and you can use it very simply:

return Integer.toString(RoboID) + DIRECTION_CHARS.get(direction);

If you want to stick with the switch statement, I wouldn't add an initial value to the variable as others have suggested - I'd throw an exception in the default case:

@Override public String toString() {
    char dirArrow;
    switch (direction) {
        case UP:
            dirArrow= '^';
            break;
        case RIGHT:
            dirArrow= '>';
            break;
        case DOWN:
            dirArrow= 'V';
            break;
        case LEFT:
            dirArrow= '<';
            break;
        default:
            throw new IllegalStateException("Unexpected direction! " + direction);
    }
    return (Integer.toString(RoboID) + dirArrow);
}

That's assuming it really isn't valid to have a direction other than the ones you've listed. Now, if you add a direction and forget to update this method, you'll end up with an exception rather than silent bad data.

The reason to not use a "dummy" initial value for the variable is that it doesn't express your desired behaviour. If you were to describe the method to someone in words, you would never mention that value, unless you really have a default value (e.g. space), in which case it's absolutely the right way to go.

I think it should be like `doNothing` for `default`. Because other than specified keys none is an action key. — Ravinder Reddy, Apr 29 '14 at 11:38
@Ravinder: It's not clear what you mean, but it looks to me like the OP *expects* `direction` to be one of those enum values. — Jon Skeet, Apr 29 '14 at 11:40
Yes I understand that. But not sure, unless it is an action key, how OP is going to use the return value. — Ravinder Reddy, Apr 29 '14 at 11:41
@Ravinder: So why would you assume that the right return value would be `doNothing`? If the system isn't in an expected state, I'd normally prefer an exception so I can find out about that quickly... — Jon Skeet, Apr 29 '14 at 11:42

score 0 · Answer 6 · answered Apr 29 '14 at 11:32

0

A better solution is to change the enum

enum Direction {
    UP("^"), DOWN("v"), LEFT("<"), RIGHT(">");
    public final String arrow;
    private Direction(String arrow) {
       this.arrow = arrow;
    }
}

// in your Robot class.
public String toString(){
    return RoboID + direction.arrow;
}

BTW I would use roboID for a field name.

answered Apr 29 '14 at 11:32

Peter Lawrey

525,659
79
751
1,130

Would you really make it a public field rather than introducing a method? – Jon Skeet Apr 29 '14 at 11:43

case switch java compile error

6 Answers6