1

In our code base, I see a lot of code like this

var error = ValidatePhoneNumber(userId, phoneNumber);

//if validation fails, return error
if(!string.IsNullOrEmpty(error))
{
    return error;
}

If I were writing this, I would have just had ValidatePhoneNumber(userId, phoneNumber) throw the error and call the method inside of a try/catch block. The only reason I can think of for this kind of practice is that throwing/catching errors can become expensive. Would concerns of performance really necessitate this kind of error handling? Or would a try/catch pattern be more useful in this situation?

I've seen posts like this, and I can see that there are some minor performance differences in including exceptions, but are they large enough to concern most applications?

(I work mostly in the .NET framework, if that makes any difference to the answer)

16
  • 3
    Exception = an act/case which should not happened.
    – Fabio
    May 3, 2017 at 22:14
  • Not an answer, but this particular example may well not be an exception- it's the validation failing because the content isn't valid.
    – user232573
    May 3, 2017 at 22:15
  • 1
    @Fabio Some systems use Exceptions as best practice for handling any sort of error. cough Microsoft cough See msdn.microsoft.com/en-us/library/ms173163.aspx for example
    – Peter M
    May 3, 2017 at 22:20
  • @PeterM, from your link: Exceptions should not be used to change the flow of a program as part of ordinary execution. Exceptions should only be used to report and handle error conditions.
    – Fabio
    May 3, 2017 at 22:22
  • @Fabio So then it comes down to Is the phone number expected to be valid at this point, or is it an error that isn't? There is not enough context at this point
    – Peter M
    May 3, 2017 at 22:24

5 Answers 5

9

Performance is not a reason to do any of this.

If you are returning errors, rather than throwing them, I hope you at least do it consistently.

The reason why has nothing to do with performance, language, or program correctness. It has to do with confusing humans.

If you offer some functions/methods to me that I can use to solve problems don't confuse me by having some of them return errors and some throw exceptions. Pick one style and stick with it!

That doesn't mean this code is necessarily wrong. It had just better not be in a context where functions are expected to throw. It had better be returning to a context where functions are expected to return errors.

I can't tell if this code is good or bad since I only have this one function to go on. But it certainly raises my eyebrows. What I need to see is what the other methods/functions are doing. If this is doing things different from the others around it then it's going to be a nasty surprise.

If you think the reason we aren't supposed to use exceptions for flow control is because grabbing a stack trace is too expensive then you've never heard of Knuth.

The reason you're not supposed to use exceptions for flow control is because they're almost as hard to follow as goto. So use them for when things go wrong. Use them to move error handling logic away from the "sunny day" logic. Don't use them because you think they're a fun way to branch your code. Don't amaze me with your clever ways of using them. I have enough to think about.

3

As C# programming guide stays: Things to Avoid When Throwing Exceptions

Exceptions should not be used to change the flow of a program as part of ordinary execution. Exceptions should only be used to report and handle error conditions.

Validation is part of "ordinary" execution logic in your application.
If you wrap your validation method in try .. catch block, what type of exception you will going to catch, Exception? If so, then what happens if validation method thorw exception not related to validation?
Using exceptions as validation logic can be misleading for other developers or readers of your code (most of the time programmers read code).

I am pretty sure that validation can be done in more readable way then using exceptions.

var phoneNumber = ValidatePhoneNumber(string number);
var outputText = phoneNumber.ToText();

Where

public interface IPhoneNumber
{
    string ToText();
}

public class ValidPhoneNumber
{
    public ValidPhoneNumber(string number) { _number = number } 
    public string ToText()
    {
        return $"Phone: {number}";
    }
}

public class InvalidPhoneNumber
{
    public ValidPhoneNumber(string errorMessage) { _message = errorMessage} 
    public string ToText()
    {
        return _message;
    }
}

And validate method

public IPhoneNumber ValidatePhoneNumber(string rawNumber)
{
    // do your validation logic get error message if possible

    if (IsValid)
    {
        return new ValidPhoneNumber(validNumber);
    }
    else
    {
        return new InvalidPhoneNumber(errorMessage);
    }
}

With approach above, by introduced InvalidPhoneNumber you hide "implementation" details about validation and just use "invalid" implementation of interface.

6
  • "Validation is part of "ordinary" execution logic" as an absolute statement is not always true. "Validation" can encompass a wide variety of checks depending on your applications, and for some situations a validation very rarely fails, such as checking data received from other components, not just stuff inputted by an operator. In those situations using exceptions can be appropriate. May 3, 2017 at 22:58
  • One step further and you end up with a well-known Result<V, E> class, which either wraps a value of type V, or an error of type E (usually a string). The great Railway-Based Programming presentation explains it in detail.
    – 9000
    May 4, 2017 at 2:57
  • Why should exceptions only be used for "out-of-ordinary" situations - is it because of performance?
    – user253751
    May 4, 2017 at 3:00
  • 3
    @immibis: You might be interested in Martin Fowler's opinion on this topic. He favors the 'exceptions are for exceptional situations' approach, too.
    – 9000
    May 4, 2017 at 4:12
  • @immibis -simply, because of the same reason as you don't use string for storing integers - use right tool for the job.
    – Fabio
    May 4, 2017 at 6:04
0

Exceptions, as the name says, should be used for exceptional circumstances.

Validating user input and finding it at fault is in no way exceptional. At least not with the users I know. They make mistakes all the time. So handling them is the normal thing the program has to do.

Now if your phone number came from a super secure database and should by all means be valid, then checking it again and finding it invalid should be an exception.

So this really is not a technical decision. Ask yourself if the error condition is normal, or an exception.

Lets take another example: the method IsMyPizzaDoneYet() returns a bool. Simply because there is a very real chance you were impatient and it's not done yet. Nothing to see, move along. It might as well throw a HolyShitTheOvenIsOnFireException if the oven is on fire. Because that's an exception.


To not confuse people, if the point of a method is throwing an exception, just name it so. Building on the example above, where suddenly finding that your database is inconsistent and lacks a valid phone number despite the fact that you checked them when users are created:

public void SendTextMessageToKnownUser(int userID, string message)
{
   var phone = GetPhoneNumberFromSecureDatabase(userID);

   ThrowIfNotValid(phone);

   phone.Send(message);
}
0

To highlight the performance aspect of the question:


My obligatory reference: Fix it you should: Are you aware that you have thrown over 40,000 exceptions in the last 3 hours?

Executive Summary: In .NET, throwing an exception is expensive. Don't do it on regular hot code paths.


The takeaway is still the same, exceptions should be exceptional, so if you validate input, using an exception might be a performance problem if the method to validate the input isn't used in the GUI, but suddenly is used to validate, e.g., an input csv file with 1M records. Better not use exceptions for every wrong record there.

That being said, my take would be roughly:

void ValidatePhoneNumberFromInteractiveContext(...) {
  ValidatePhoneNumber_Throws(userId, phoneNumber);
  // The interactive caller knows how to format and display
  // exceptions
}

/or/

ErrorObject ValidatePhoneNumberFromBatchContext(...) {
  return ValidatePhoneNumber_WithError(userId, phoneNumber);
}

Do note that the first approach only really works properly if your exception contains sufficient information to build up a decent "message" at the catch site.

0

Is the product you are working on derived form work that was done in C, or was designed to work with C.

If so, then it is not for performance reasons why exceptions are not being used.

In the example that you give, it doesn't really matter how errors are handled, just that they are handled consistently.

If the person telling you this is your technical lead or anyone you don't lead, don't cotradict him. Just realize it is for other reasons, you will eventually need their help.

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.