Throw Exception VS Return Error within a Try,Catch,Finally

hugoware picture hugoware · Jul 10, 2009 · Viewed 19.7k times · Source

I'm pretty sure I already know the answer, but I'm still curious what the opinion is on handling an error within a Try,Catch,Finally block -- but when you're repeating yourself.

BTW - I'm not talking about User Input - but using that for an example because it is clear and short

Consider this bit of code...

try {    
    if (success) {
        return someSuccessMessage;
    }
    else {
        logError("User input not correct format");
        return someErrorMessage; // repeats itself
    }
}
catch (Exception ex) {
    logError(ex.Message);
    return someErrorMessage; // repeats itself
}

Say we have a function, that if it fails we want to return an error message because the exception is irrelevant -- our function did not succeed and the user doesn't need any additional detail.

I've always held the belief that if you can handle the error, avoid the exception -- since it isn't exceptional anymore, but I wondered the opinion about avoiding repeating yourself as well... You could do the following to avoid repeating yourself...

try {    
    if (success) {
        return someSuccessMessage;
    }
    else {
        throw new Exception("User input not correct format");
    }
}
catch (Exception ex) {
    logError(ex.Message);
    return someErrorMessage;
}

This isn't the best example, but I was going for brevity to make the point of repeating code.

Exceptions are known to incur a performance penalty, but what are the thoughts about a situation like this?

Answer

John Saunders picture John Saunders · Jul 10, 2009

I question the separation of concerns here. Unless this function is part of the UI, it should not concern itself with error messages. It should be throwing exceptions instead. The caller of this method, if it's part of the UI, might want to produce an error message for display. If the caller were a web service, then it would want to produce a SOAP Fault, which might not use the same message (if it used any message at all).

I would also strongly suggest you log ex.ToString() and not ex.Message.