Using exceptions for error handling flow?

Question

I have the following pattern:

private void MyFunction()
{
    doStuff();

    if (problem())
    {
        cleanup();
        return;
    }

    doMoreStuff();

    if (otherProblem())
    {
        cleanup();
        return;
    }

    doYetMoreStuff();
}

The error cleanup code is duplicated. The obvious way to eliminate this is:

private void MyFunction()
{
    try {
        doStuff();

        if (problem()) throw new MyException();

        doMoreStuff();

        if (otherProblem()) throw new MyException();

        doYetMoreStuff();
    }
    catch (MyException)
    {
        cleanup();
        return;
    }
}

However, the error cases are not really exceptional - this is an ASP.Net page, and bad or no data in the query string will trigger the error cases. Exceptions seem to be the most obvious way to deduplicate the error handling and separate it from the main code, but as I understand it the accepted best practice is not to use exceptions for control flow like this.

What is the customary way of doing this?

Well the code isn't being duplicated in the first instance if you use that cleanup() method? As you said, exceptions should only be used for exceptional cases. — Dave New
– Dave New, Commented Jul 5, 2012 at 13:38

Sergey Kalinichenko · Accepted Answer · 2012-07-05 13:38:24Z

3

Using exceptions for non-exceptional situations is not a great idea, especially when the exception is thrown and caught in the same method. A better approach would be to use a boolean variable that indicates a need for cleanup, and performing your cleanup inside a finally block.

var needsCleanup = true;
try {
    doStuff();

    if (problem()) return;

    doMoreStuff();

    if (otherProblem()) return;

    doYetMoreStuff();

    needsCleanup = false;
} finally {
    if (needsCleanup) {
        cleanup;
    }
}

answered Jul 5, 2012 at 13:38

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Sergey Kalinichenko Over a year ago

@Steve Thanks, performance hit is definitely a valid concern when using exceptions to pass control.

StuartLC · Accepted Answer · 2012-07-05 13:40:15Z

1

You could use just try .. finally and early returns.

private void MyFunction()
{
  try
  {
    doStuff();

    if (problem())
    {
        return;
    }

    doMoreStuff();

    if (otherProblem())
    {
        return;
    }

    doYetMoreStuff();
  }
  finally
  {
    cleanup();
  }
}

answered Jul 5, 2012 at 13:40

StuartLC

108k18 gold badges225 silver badges295 bronze badges

Comments

Steven Doggart · Accepted Answer · 2012-07-05 13:40:44Z

1

The finally block will be executed any time execution leaves the try block, even if the reason that it leaves the try block is because you call return. So, you could do something like this:

private void MyFunction()
{
    try
    {
        doStuff();
        if (problem()) return;

        doMoreStuff();
        if (otherProblem()) return;

        doYetMoreStuff();
    }
    finally
    {
        cleanup();
    }
}

answered Jul 5, 2012 at 13:40

Steven Doggart

43.8k8 gold badges71 silver badges109 bronze badges

Comments

Jakub Konecki · Accepted Answer · 2012-07-05 13:37:05Z

0

If cleanup() is really just a method call than it's not duplicated.

I wouldn't use exceptions in this case. You can easily google for articles describing in details why using exception in normal flow is a bad idea: mainly in impacts performance and can screw up performance counters ('# of exceptions thrown' will be meaningless).

answered Jul 5, 2012 at 13:37

Jakub Konecki

46.1k8 gold badges92 silver badges128 bronze badges

1 Comment

Thom Smith Over a year ago

cleanup is about three lines.

Filip Ekberg · Accepted Answer · 2012-07-05 13:39:56Z

0

Exceptions should be thrown when something unexpected happens.

Consider that you want to add a user in your database, but if the username already exists, you just want a error saying so. Now, an error does not have to be an exception!

bool AddUser(string username)

Now the flow would look like this instead:

if(AddUser(username)) {}
else
{
    // Notify the user that it didn't work
}

If an exception was thrown here, it Should be because the software couldn't connect to the database, not because the username already existed.

throwing and catching exceptions for flow control like this will cause more overhead than needed and thus you should use if/else to control this instead.

answered Jul 5, 2012 at 13:39

Filip Ekberg

36.3k20 gold badges129 silver badges184 bronze badges

Collectives™ on Stack Overflow

Using exceptions for error handling flow?

5 Answers 5

1 Comment

Comments

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related