Exception Handling a switch method

Question

I'm trying to get a program I made to ask someone for a number between 1 and 13 in order to get them to make a selection. I'm trying to figure out how to handle if they want to be contrarian and enter a non-valid number or a character or string. Here's what I have so far...

try {
        attackingUnit = selectUnit(input.nextInt());
        attackerUnitName = attackingUnit.getUnitName();
    } catch (NullPointerException e) {
        System.out.println("Invalid option, please pick a valid option\n");
        showUnitSelection();
        attackingUnit = selectUnit(input.nextInt());
        attackerUnitName = attackingUnit.getUnitName();
}

Here's the method I'm using for a making the selection itself:

private static Unit selectUnit(int selection) {

    switch (selection) {
        case 1:
            return Unit.GreatSwords;
        case 2:
            return Unit.BlackOrcs;
        case 3:
            return Unit.Bestigor;
        case 4:
            return Unit.ChaosChosen;
        case 5:
            return Unit.MenAtArms;
        case 6:
            return Unit.Executioners;
        case 7:
            return Unit.GraveGuard;
        case 8:
            return Unit.Retributors;
        case 9:
            return Unit.StormVermin;
        case 10:
            return Unit.SwordMasters;
        case 11:
            return Unit.TombGuard;
        case 12:
            return Unit.WildWoodRangers;
        case 13:
            return Unit.Hammerers;
    }

    return null;
}

I'm pretty sure I'm not doing this right, if you don't mind I'd like to hear some suggestions to consider.

add a default case returning the Unit.Invalid if no other case match and check if it's invalid and show error to the user. — Vikram Palakurthi, Mar 30 '18 at 19:00
Use the default case, and then add a if statement to check if the default case was returned and if it was then you know its illegal input — SteelToe, Mar 30 '18 at 19:00
why you catch NullPointerException, it can be IllegalArgumentException . — Afgan, Mar 30 '18 at 19:02
@Dan I personally think it is more preferable to use a default case with a if statement as exception handling uses more resources and becomes more messy. See my answer below which avoids all exception handling — SteelToe, Mar 30 '18 at 19:12
`null` is not a `NullPointerException` - you only get an exception if you try to use it. You check for `null` with `== null`. — Bernhard Barker, Mar 30 '18 at 19:17

score 1 · Answer 1 · answered Mar 30 '18 at 19:21

I think code will look much prettier if you do it in this way:

public enum Unit{

        GreatSwords(1),
        BlackOrcs(2),
        Bestigor(3),
        ChaosChosen(4),
        MenAtArms(5),
        Executioners(6),
        GraveGuard(7),
        Retributors(8),
        StormVermin(9),
        SwordMasters(10),
        TombGuard(11),
        WildWoodRangers(12),
        Hammerers(13)

        private int index;

        public int getIndex() {
            return this.index;
        }

        public static getUnitByIndex(int index) throws IllegalArgumentException {
            return Stream.of(values())
            .filter(unit -> unit.getIndex() == index)
            .findFirst()
            .orElseThrow(() -> new IllegalArgumentException("Invalid value");
        }

}

score 0 · Answer 2 · answered Mar 30 '18 at 19:04

The idea over here would be to avoid nulls, therefore the best option would be to make a default case that return Unit.Unknown if the switch statement does not match anything. Then you can check if the input equals Unit.Unknown and if it does tell the user that it is invalid.

To do this, First add in a "Unknown" member to the Unit enum.

Then add in a default case to your switch statement like this:

     default:
        return Unit.Unknown;

Then in your original code change it to this:

       attackingUnit = selectUnit(input.nextInt());
    attackerUnitName = attackingUnit.getUnitName();
    //check if its valid
    if(attackerUnitName==Unit.Unknown){
    //unit is unknown ,tell the user the input was invalid
    } else {
    //input was valid, do what you want to do
    }

Why is `Unit.Unknown` better than `null`? They seem functionally pretty similar to me (especially if the use case is to just instantly check and replace it), which I'd argue makes `null` the better option. — Bernhard Barker, Mar 30 '18 at 19:18

score 0 · Answer 3 · answered Mar 30 '18 at 19:05

Your switch code would be like with default case-

    private static Unit selectUnit(int selection) {

        switch (selection) {
            case 1:
                return Unit.GreatSwords;
            case 2:
                return Unit.BlackOrcs;
            case 3:
                return Unit.Bestigor;
            case 4:
                return Unit.ChaosChosen;
            case 5:
                return Unit.MenAtArms;
            case 6:
                return Unit.Executioners;
            case 7:
                return Unit.GraveGuard;
            case 8:
                return Unit.Retributors;
            case 9:
                return Unit.StormVermin;
            case 10:
                return Unit.SwordMasters;
            case 11:
                return Unit.TombGuard;
            case 12:
                return Unit.WildWoodRangers;
            case 13:
                return Unit.Hammerers;
 default:throw new IllegalArgumentException(""Invalid option, please pick a valid option\n");
        }

    }

Your handling code would be like -

try {
        attackingUnit = selectUnit(input.nextInt());
        attackerUnitName = attackingUnit.getUnitName();
    } catch (IllegalArgumentException e) {
        System.out.println(e.getMessage);
        showUnitSelection();
        attackingUnit = selectUnit(input.nextInt());
        attackerUnitName = attackingUnit.getUnitName();
}

Related: [When to throw an exception?](https://stackoverflow.com/q/77127) — Bernhard Barker, Mar 30 '18 at 19:21
good to [throw exception](https://stackoverflow.com/questions/2567932/throwing-exception-vs-returning-null-value-with-switch-statement) instead returning null — Afgan, Mar 30 '18 at 19:23

Abi · Answer 4 · 2018-03-30T19:42:30.420

Try this. Using exception handling for simple validation is an expensive operation, although it will work just fine.

attackingUnit = null;
while(attackingUnit == null){
   System.out.println("Invalid option, please pick a valid option\n");
   showUnitSelection();
   attackingUnit = selectUnit(input.nextInt());   
}

attackerUnitName = attackingUnit.getUnitName();

Update: Modify you method to include default case (as per @SteelToe suggestion)

private static Unit selectUnit(int selection) {

switch (selection) {
    case 1:
        return Unit.GreatSwords;
    case 2:
        return Unit.BlackOrcs;
    case 3:
        return Unit.Bestigor;
    case 4:
        return Unit.ChaosChosen;
    case 5:
        return Unit.MenAtArms;
    case 6:
        return Unit.Executioners;
    case 7:
        return Unit.GraveGuard;
    case 8:
        return Unit.Retributors;
    case 9:
        return Unit.StormVermin;
    case 10:
        return Unit.SwordMasters;
    case 11:
        return Unit.TombGuard;
    case 12:
        return Unit.WildWoodRangers;
    case 13:
        return Unit.Hammerers;
    default:
        return Unit.Unknown;

  }


}

And do this:

attackingUnit = Unit.Unknown;
while(attackingUnit == Unit.Unknown){
   System.out.println("Invalid option, please pick a valid option\n");
   showUnitSelection();
   attackingUnit = selectUnit(input.nextInt());   
}

attackerUnitName = attackingUnit.getUnitName();

@SteelToe not really. I was just try to give a simple idea as I assuming he was trying a validation loop until user key in the correct input. Ofcourse you are right and that is the better way to do. — Abi, Mar 30 '18 at 19:12
@SteelToe: Yeah I was trying to make it so that the user had to enter a valid option. — Dan, Mar 30 '18 at 19:23

Exception Handling a switch method

4 Answers4