Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Throwing exceptions in switch statements when no specified case can be handled

Let's say we have a function that changes a password for a user in a system in an MVC app.:

public JsonResult ChangePassword
    (string username, string currentPassword, string newPassword)
{
    switch (this.membershipService.ValidateLogin(username, currentPassword))
    {
        case UserValidationResult.BasUsername:
        case UserValidationResult.BadPassword:
            // abort: return JsonResult with localized error message        
            // for invalid username/pass combo.
        case UserValidationResult.TrialExpired
            // abort: return JsonResult with localized error message
            // that user cannot login because their trial period has expired
        case UserValidationResult.Success:
            break;
    }

    // NOW change password now that user is validated
}

membershipService.ValidateLogin() returns a UserValidationResult enum that is defined as:

enum UserValidationResult
{
    BadUsername,
    BadPassword,
    TrialExpired,
    Success
}

Being a defensive programmer, I would change the above ChangePassword() method to throw an exception if there's an unrecognized UserValidationResult value back from ValidateLogin():

public JsonResult ChangePassword
    (string username, string currentPassword, string newPassword)
{
    switch (this.membershipService.ValidateLogin(username, currentPassword))
    {
        case UserValidationResult.BasUsername:
        case UserValidationResult.BadPassword:
            // abort: return JsonResult with localized error message        
            // for invalid username/pass combo.
        case UserValidationResult.TrialExpired
            // abort: return JsonResult with localized error message
            // that user cannot login because their trial period has expired
        case UserValidationResult.Success:
            break;
        default:
            throw new NotImplementedException
                ("Unrecognized UserValidationResult value.");
            // or NotSupportedException()
            break;
    }

    // Change password now that user is validated
}

I always considered a pattern like the last snippet above a best practice. For example, what if one developer gets a requirement that now when users try to login, if for this-or-that business reason, they're suppose to contact the business first? So UserValidationResult's definition is updated to be:

enum UserValidationResult
{
    BadUsername,
    BadPassword,
    TrialExpired,
    ContactUs,
    Success
}

The developer changes the body of the ValidateLogin() method to return the new enum value (UserValidationResult.ContactUs) when applicable, but forgets to update ChangePassword(). Without the exception in the switch, the user is still allowed to change their password when their login attempt shouldn't even be validated in the first place!

Is it just me, or is this default: throw new Exception() a good idea? I saw it some years ago and always (after groking it) assume it to be a best practice.

like image 510
CantSleepAgain Avatar asked Jul 28 '10 02:07

CantSleepAgain


People also ask

Can a switch statement have no cases?

The switch statement can include any number of case instances. However, no two constant-expression values within the same switch statement can have the same value.

Can switch-case throw exception in Java?

If an exception occurs inside the switch, then it's fine. The one reasonably standard place you might throw inside a switch statement is in the default case. If reaching that case indicates invalid input, then it's reasonable to throw an exception.

How do I add exceptions to a switch-case?

in switch, use default: \\ throw an exception. To throw a exception in a switch statement you throw the exception. The only slight surprise is that if the case only throws an exception -- no if logic -- then the break will be dead if you use it and the compiler will complain. So skip the break .

What will happen if the switch statement did not match any cases?

If the switch condition doesn't match any condition of the case and a default is not present, the program execution goes ahead, exiting from the switch without doing anything. Show activity on this post.


4 Answers

I always throw an exception in this case. Consider using InvalidEnumArgumentException, which gives richer information in this situation.

like image 126
Matt Greer Avatar answered Oct 02 '22 18:10

Matt Greer


With what you have it is fine although the break statement after it will never be hit because execution of that thread will cease when an exception is thrown and unhandled.

like image 35
Jesus Ramos Avatar answered Oct 02 '22 18:10

Jesus Ramos


I always like to do exactly what you have, although I usually throw an ArgumentException if it's an argument that was passed in, but I kind of like NotImplementedException better since the error is likely that a new case statement should be added rather than the caller should change the argument to a supported one.

like image 44
Davy8 Avatar answered Oct 02 '22 18:10

Davy8


I've used this practice before for specific instances when the enumeration lives "far" from it's use, but in cases where the enumeration is really close and dedicated to specific feature it seems like a little bit much.

In all likelihood, I suspect an debug assertion failure is probably more appropriate.

http://msdn.microsoft.com/en-us/library/6z60kt1f.aspx

Note the second code sample...

like image 27
Rob Cooke Avatar answered Oct 02 '22 18:10

Rob Cooke