Best Practice for Try Catch Error Handling

Question

I'm trying to avoid returning an incorrect value when in the catch but I'm having trouble finding a better solution than this:

    private SecurityLevel ApiGetSecurityLevel()
    {
        try
        {
            return _BioidInstance.GetSecurityLevel();
        }
        catch
        { 
            return SecurityLevel.High;
        }
    }

Is there a better way of doing this so I don't return incorrect values? I can't change the SecurityLevel enum.

Answer 1

You could return Nothing. And make your function look at the result as If NOT ApiGetSecurityLevel() Is Nothing

Answer 2

Do not catch the exception. Allow the exception to "bubble-up" to force the caller/callers to handle setting the default security value.

---

If you really want to return a value then use Nullable<SecurityLevel> or SecurityLevel?.

private SecurityLevel? ApiGetSecurityLevel() { 
    try { 
        return _BioidInstance.GetSecurityLevel(); 
    } 
    catch {  
        return null; 
    } 
} 

Then use as:

if (ApiGetSecurityLevel().HasValue == false) {
    // use default security level
}

Answer 3

Is it possible this is a case where the application should just fail? That is, if the SecurityLevel can't be determined, the user shouldn't be able to continue?

If so, why no just re-throw and let the UI handle it (or let it get logged, however your shop works)?

If the application should be able to continue, pick (and document) some default value and do exactly what you're doing.

Answer 4

You could just let the exception bubble up.

Answer 5

You could wrap it inside one of your own enum, and translate theirs to yours. Then yours could contain an "invalid" security level and you would avoid using blackboxed types.

Answer 6

In addition to Matt's answer, whats wrong with letting the exception happen? If there is some invalidity of state or access in your application, it would probably be better to just kill it.

Answer 7

If you can change the return type of the fonction, I'll change it to a nullable enum and return null in the catch.

private SecurityLevel? ApiGetSecurityLevel()
{
    try
    {
        return _BioidInstance.GetSecurityLevel();
    }
    catch
    { 
        return null;
    }
}

Answer 8

Firstly, there's no reason for that try/catch as long as GetSecurityLevel returns a SecurityLevel. The compiler will catch any problems there.

Secondly, that's not a good use of try/catch. Try/catch should never be used for normal control flow, only for exceptional cases.

If, for some reason, GetSecurityLevel() does not return a SecurityLevel enum type:

private SecurityLevel ApiGetSecurityLevel()
    {
        object securityLevel = _BioidInstance.GetSecurityLevel();
        if (securityLevel is SecurityLevel)
        {
             return _BioidInstance.GetSecurityLevel();
        }
        else
        {
             throw new Exception("Invalid SecurityLevel");
        }
    }

Answer 9

public class BioId { public SecurityLevel SecLevel { get; set; } }

private bool ApiGetSecurityLevel(BioId bioId)
{
    try
    {
        bioIdSecLevel = bioId.GetSecurityLevel();
        return true;
    }
    catch (PossibleException)
    {
        return false;
    }
}

Then use it with an if and if it returns false, then let the handling code decide the default securitylevel for it's BioId.

Though I agree with everyone else that in practicality you want to just let the exception bubble and let top levels handle them. Though sometimes there are standards not to let exceptions bubble because each layer they bubble through can apparently hit performance, but to that I say you shouldn't have too many layers anyway, onions are complex, like ogres.

Answer 10

If you can add a Nothing/NotSet enumeration to SecurityLevel, then you can return that from the catch block. Returning escalated privs when you can't determine them is a really strange choice.

Alternatively, return SecurityLevel as a nullable enumeration.

private SecurityLevel? ApiGetSecurityLevel()
{
    try
    {
        return _BioidInstance.GetSecurityLevel();
    }
    catch
    { 
        return null;
    }
}

Answer 11

SecurityLevel sec = _BioidInstance.GetSecurityLevel();
return (Enum.IsDefined(Typeof(SecurityLevel), sec) ? sec : SecurityLevel.High);

It will return the security level in _BioidInstance if it is defined in the enum, otherwise it will return SecurityLevel.High.

If it were me, if I knew that the security level can't be anything outside of what is defined in the enum, I would just do without the try-catch and let it fail if it comes to it. That way I know something else somewhere went awry and I can go fix it.

Answer 12

In this case, security becomes binary.

To use an example of a secure compound, at the gate either someone can get in or they can't. If they fail to get a security level, that person should not get thru the gate.

If the GetSecurityLevel() method throws an exception, something was turned away at the gate. To then grant some arbitrary security level and allow them thru is not a good idea.

I would do away with this method completely, and replace it with something closer to

private bool HasSecurityLevel(SecurityLevel securityLevel) 
{ 
    try 
    { 
        return _BioidInstance.GetSecurityLevel() == securityLevel; 
    } 
    catch 
    {  
        return false; 
    } 
} 

and then check for each security level on an as needed basis.

The rational for this is at some point the check has to be made, may as well make it as close to the source (when first getting security level) as possible.