Using exceptions to control program flow

Question

Let me give you an example. I have the following web method inside my aspx.cs file which I use for AJAX calls:

[WebMethod]
public static ResponseMessage GetNextQuestion(string quizGuid)
{
    using (DbEntities db = new DbEntities())
    {
        Quiz theQuiz = Quiz.Get(db, DataValidationHelper.GetGuid(quizGuid));

        try
        {
            Question nextQuestion = QuizHelper.GetNextQuestion(db, theQuiz);

            return new ResponseMessage() { Status = "Success", NextQuestion = new NextQuestionResponse(nextQuestion, theQuiz) };
        }
        catch (QuizNotFoundException)
        {
            return new ResponseMessage() { Status = "QuizNotFound" };
        }
        catch (QuizInvalidException)
        {
            return new ResponseMessage() { Status = "QuizInvalid" };
        }
        catch (QuizOverException)
        {
            return new ResponseMessage() { Status = "QuizOver" };
        }
        catch (QuestionTimedOutException)
        {
            return new ResponseMessage() { Status = "QuestionTimedOut" };
        }
        catch (Exception ex)
        {
            return new ResponseMessage() { Status = "Error", ErrorMessage = ex.Message };
        }
    }
}

The QuizHelper.GetNextQuestion method generates a new question from the database and in some specific cases throws the following exceptions:

1. QuizNotFoundException: When a quiz with the given quizGuid is not found in the database.
2. QuizInvalidException: Thrown for security purposes, for example when someone tries to hack HTTP requests.
3. QuizOverException: Every quiz has 10 questions and when the user tries to get 11th question using the QuizHelper.GetNextQuestion method, this exception is thrown.
4. QuestionTimedOutException: You have to answer a question within a given time. If you don't, this exception is thrown.
5. Exception: All other exceptions are grouped under this for the sole purpose of informing the user that an error has occured, for UX purposes.

Then inside the Javascript file, the ResponseMessage.Status is checked and corresponding action is taken.

I know that using exceptions as they are used in this code to control the flow is bad but making it this way is more intuitive and is much simpler. Not to mention the fact that the code is easier to understand for an outsider.

I am not sure how this code could be rewritten in the "right way" without exceptions but at the same time conserving its simplicity.

Am I missing something, any ideas?

UPDATE: Some answers propose using an Enum to return the status of the operation but I have lots of operations and they all may result in different scenarios (i.e. I cannot use the same Enum for all operations). In this case creating one Enum per operation does not feel to be the right way to go. Any improvements over this model?

Answer 1

You would e. g. use an enum Quiz.Status and change

Question nextQuestion = QuizHelper.GetNextQuestion(db, theQuiz)

to

Question nextQuestion;
Quiz.Status status = QuizHelper.TryGetNextQuestion(db, theQuiz, out nextQuestion);
switch(status) {
    case QuizNotFoundException:
        return new ResponseMessage() { Status = "QuizNotFound" };
    // ...
}

or even simplify that to

Question nextQuestion;
Status status = QuizHelper.TryGetNextQuestion(db, theQuiz, out nextQuestion);
if(status != Status.ok) {
    return new ResponseMessage() (Status = status);
}

avoiding the long cascade of different cases.

Answer 2

Throwing and handling exceptions are expensive. You are free to throw exceptions when invalid argument is passed or an object is about to enter invalid state, etc. Here it seems you're using exceptions in regular program flow.

Microsoft suggests not to use exceptions to alter the program flow.

While the use of exception handlers to catch errors and other events that disrupt program execution is a good practice, the use of exception handler as part of the regular program execution logic can be expensive and should be avoided. In most cases, exceptions should be used only for circumstances that occur infrequently and are not expected.. Exceptions should not be used to return values as part of the typical program flow. In many cases, you can avoid raising exceptions by validating values and using conditional logic to halt the execution of statements that cause the problem.

Here is a code analysis rulewhich states that.

So, In your case you need to return some sort of ErrorCode as Enum or int. Will be much better or wrap your ErrorCode in QuizException in case you feel you need an exception to be thrown here!

Edit: Based on your edit, I think you can very much do it. Why creating an Enum or Int as error code is not possible? For instance take an example of WindowsSocket which exposes SocketErrorCodes holds all kind of socket errors. Or even more appropriate one here is Operating System itself uses Unique SystemErrorCodes which can be wrapped in an Enum Isn't it?

Correct me if am wrong or am missing your point!

Answer 3

There is no any need here to manage flow via exception (apart may be Exception and TimeOutException ). Remember that exception as itself a heavy enough artifact, and not suitable for fluid control flows and also as name suggests: it's for exceptional situations.

There are places where you simply can not avoid exception baced flow, like for example: when you are working with devices. It's a very common command behavior based on exceptions recieved from device or IO.

But, as I said, it doesn't seem to be your case, so just handle it with simple if/else, whenever it's possible.

Answer 4

It is "expensive" when a exception is thrown since several calls is done when a Exception is raised.

I would instead recommend a Question.Status or Quiz.Status that is a Enum.

Edit: For more info Why are try blocks expensive