Purposely raising exceptions to use catch

Question

For a typical if...else wrapped with exception handling, is something like the following example a recommended practice to avoid code duplication?

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

instead of...

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        return null;
    }
}
catch(Exception ex)
{
    return null;
}

I know there is a slight performance hit, but I'm wondering if this is considered to be an acceptable practice. I currently do it the second method - especially in cases where I need to handle specific exceptions differently - but I was wondering if the first method is appropriate for simple cases.

Answer 1

Using exception handling for flow control is discouraged by Microsoft.

And a round table on the topic is available.

That being said, C# supports doing so, and I suppose it depends on the condition encountered whether an exception is the most appropriate response.

Answer 2

The performance hit is most probably negligible, as explained in this answer.

So let's go with the idea that performance isn't an issue. You're throwing System.Exception, just to move execution into the catch clause. Throwing a BadControlFlowThatShouldBeRewrittenException would probably be overkill though.

Let's break this down. We have:

• Method GetDataFromServer (method names should be PascalCase in C#), which can possibly throw an exception, or return a bool.
• If result was true, run ProcessData.
• Return null otherwise.

Looks like the method where this code is written, is simply doing too many things. GetDataFromServer returning a bool looks like a design flaw, I'd be expecting that method to return the data it's getting from the server, some IEnumerable<SomeType> that would contain 0 or more items - i.e. happy path returns n items where n > 0, not-so-happy path returns 0 items, and unhappy path blows up with an unhandled exception, whatever that is.

That changes what the method looks like, quite a lot - again it's hard to tell whether this makes sense, because the original post only has one exit point (and thus wouldn't compile, as not all code paths return a value), so this is only a wild guess:

try
{
    var result = GetDataFromServer();
    return ProcessData(result);
}
catch
{
    return null;
}

Here you'd look at ProcessData and see that it's iterating the result, and returns null if there's no item in the IEnumerable.

Now why is the method returning null? Server was down? Is there a bug in the query? The connection string is using the wrong credentials? Whenever GetDataFromServer blows up with an exception you're not expecting, you're swallowing it, shoving it under the carpet and returning a null value. I'd recommend catching specific exceptions in this case, and log everything else; debugging will be much easier that way.

With a general catch clause that doesn't capture the exception, it gets pretty hard to diagnose anything. I'd minimally do this instead:

catch(Exception e)
{
    return null;
}

Now you can at least break and inspect e if things go wrong.

---

TL;DR: No, throwing and catching exceptions for flow control isn't a good idea.

Answer 3

in your first answer there is a performance hit that doesn't need to be there.

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

when you exit the if statement to enter into the Catch statement when you don't have to makes the code switch directions, so to say.

if you want to return null; do it in the else statement not in a catch that is caught after being thrown from the else statement.

Probably doesn't apply to your Real code, but for the Generic code that you gave it does apply.

Standards say you shouldn't do this.

Standards say you should do it like this, (again based on Generic Code Given in OP)

if (GetDataFromServer())
{
    return ProcessData();
}
else
{
    Return null
}

and since you don't have any specific exceptions that you are catching you shouldn't even have a try catch here.

you want to see Exceptions when they occur so that you can fix the issue that creates the exception.

Answer 4

Why not the far simpler:

if (!GetDataFromServer()) return null;
ProcessData();

If an exception handler is going to exist it should be in ProcessData()