2

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?

Improve this question
5
  • Why not have these as objects, like QuestionResult, similar to your ResponseMessage. It can have a Status enumeration representing these possible states; perhaps even a message. Then you'd just have Question nextQuestion = QuizHelper.GetNextQuestion(db, theQuiz); var response = new ResponseMessage() { Status = nextQuestion.Status }; if (nextQuestion.Exists) response.NextQuestion = new NextQuestionResponse(nextQuestion, theQuiz); return response;. Then your try/catch can catch truly exceptional cases. Commented Aug 29, 2013 at 20:36
  • So you propose a mechanism similar to error codes commonly used in C/C++. It feels like this technique is old fashioned but honestly I am not sure, I need to read all answers before I decide. By the way, why didn't you post this as an answer with a chance of being the answer to the question? Commented Aug 29, 2013 at 20:44
  • Not so much error codes (really, you're kinda already doing that as part of your response). If you're invoking a method that can have many reasons for "failing", and they happen often, and they are expected to happen, then yes, if possibly you should consider refactoring it so it gracefully informs you why it couldn't work. (kind of like Int32.TryParse, only maybe it'd tell you that it failed because the input was out of range vs bad format). I won't post an answer; the "simplified" FrankPl's answer is more or less what I was aiming for. Commented Aug 29, 2013 at 20:49
  • You are definitely right about the "you're kinda already doing that as part of your response" part but I am desperate to do so when communicating with Javascript. I would gladly hear the alternative way. Also thank you for the "and they are expected to happen" part, it was quite explanatory. Commented Aug 29, 2013 at 20:56
  • 1
    Are all these different outcomes used as status of ResponseMessage? Then why not make the Enum there instead of the strings or maybe with a 1:1 mapping between the different status enum values and their string representation sent to the client? Commented Aug 29, 2013 at 21:26

4 Answers 4

Reset to default
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.

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

2 Comments

There are many methods like GetNextQuestion in my project domain and all of these methods can result in many different scenarios. So you basically propose to transform all these methods to Try...(out ...), am I right?
Yes, similar to the methods in the .net framework like int.TryParse and Dictionary.TryGetValue.
1

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!

Improve this answer

1 Comment

Regarding the "some sort of ErrorCode as Enum or int" part, I have updated the question, please notice the last part.
0

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.

Improve this answer

5 Comments

You say that the use of QuestionTimedOutException might be used but at the same time claiming that exceptions are for exceptional situations. So can timing out be defined as an exceptional situation? If yes, why can't we similarly define QuizInvalidException and other exceptions as an exceptional situation?
@Anar: Time out is not something that you clearly control (don't know if this is tru in case of OP). Often happens that timeout is raise by some service, come DB request or thing like that. Things that you have no power to control or query state.
I thought you meant QuestionTimedOutException which is a controllable time interval and which I have power to control. Sorry for misunderstanding.
any thoughts about using Enum-s or error codes for this purpose? I have updated the question, please notice the last part.
@Anar: actually yes, you can choose enums, or some other way.The point is avoid using flow with exceptions, whenever it's possible.
0

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

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.