26

While searching SO for approaches to error handling related to business rule validation, all I encounter are examples of structured exception handling.

MSDN and many other reputable development resources are very clear that exceptions are not to be used to handle routine error cases. They are only to be used for exceptional circumstances and unexpected errors that may occur from improper use by the programmer (but not the user.) In many cases, user errors such as fields that are left blank are common, and things which our program should expect, and therefore are not exceptional and not candidates for use of exceptions.

QUOTE:

Remember that the use of the term exception in programming has to do with the thinking that an exception should represent an exceptional condition. Exceptional conditions, by their very nature, do not normally occur; so your code should not throw exceptions as part of its everyday operations.

Do not throw exceptions to signal commonly occurring events. Consider using alternate methods to communicate to a caller the occurrence of those events and leave the exception throwing for when something truly out of the ordinary happens.

For example, proper use:

private void DoSomething(string requiredParameter)
{
if (requiredParameter == null) throw new ArgumentExpcetion("requiredParameter cannot be null");
// Remainder of method body...
}

Improper use:

// Renames item to a name supplied by the user.  Name must begin with an "F".
public void RenameItem(string newName)
{
   // Items must have names that begin with "F"
   if (!newName.StartsWith("F")) throw new RenameException("New name must begin with /"F/"");
   // Remainder of method body...
}

In the above case, according to best practices, it would have been better to pass the error up to the UI without involving/requiring .NET's exception handling mechanisms.

Using the same example above, suppose one were to need to enforce a set of naming rules against items. What approach would be best?

  1. Having the method return a enumerated result? RenameResult.Success, RenameResult.TooShort, RenameResult.TooLong, RenameResult.InvalidCharacters, etc.

  2. Using an event in a controller class to report to the UI class? The UI calls the controller's RenameItem method, and then handles an AfterRename event that the controller raises and that has rename status as part of the event args?

  3. The controlling class directly references and calls a method from the UI class that handles the error, e.g. ReportError(string text).

  4. Something else... ?

Essentially, I want to know how to perform complex validation in classes that may not be the Form class itself, and pass the errors back to the Form class for display -- but I do not want to involve exception handling where it should not be used (even though it seems much easier!)


Based on responses to the question, I feel that I'll have to state the problem in terms that are more concrete:

UI = User Interface, BLL = Business Logic Layer (in this case, just a different class)

  1. User enters value within UI.
  2. UI reports value to BLL.
  3. BLL performs routine validation of the value.
  4. BLL discovers rule violation.
  5. BLL returns rule violation to UI.
  6. UI recieves return from BLL and reports error to user.

Since it is routine for a user to enter invalid values, exceptions should not be used. What is the right way to do this without exceptions?

James
  • 1,788
  • 4
  • 22
  • 28
  • Winforms, webforms, wpf, silverlight? Which .NET Version? – John Saunders Aug 13 '09 at 14:39
  • Also, can you clarify about these items you want to enforce rules on? Where do they come from, etc.? When should the rules be evaluated? Is this the complex validation you mention? If not, could you elaborate on that a little? – John Saunders Aug 13 '09 at 14:40
  • The complex validation that I mention would be something along the lines of enforcing that the name of an item match a particular ruleset. The full ruleset is not described here. Rather, as an example, I've demonstrated a single rule that states an item name must begin with the letter "F". – James Aug 13 '09 at 14:59
  • The question is specific to WinForms, .NET 3.5., and is applicable to small, custom business logic layers. – James Aug 13 '09 at 15:12
  • I was just reading comments that Walter Bright has recently made on Exceptions ( https://news.ycombinator.com/item?id=28164247 ) and I got to this post. It is very interesting that the MSDN quote above -- which is very good and explains a lot -- is no longer available in the MSDN docs. – raddevus Aug 13 '21 at 13:04

8 Answers8

5

I assume that you are creating your own business rules validation engine, since you haven't mentioned the one you're using.

I would use exceptions, but I would not throw them. You will obviously need to be accumulating the state of the evaluation somewhere - to record the fact that a particular rule failed, I would store an Exception instance describing the failure. This is because:

  1. Exceptions are serializable
  2. Exceptions always have a Message property that is human-readable, and can have additional properties to record details of the exception in machine-readable form.
  3. Some of the business rules failures may in fact have been signaled by exceptions - a FormatException, for instance. You could catch that exception and add it to the list.

In fact, this month's MSDN Magazine has an article that mentions the new AggregateException class in .NET 4.0, which is meant to be a collection of exceptions that occurred in a particular context.


Since you're using Windows Forms, you should use the built-in mechanisms for validation: the Validating event and the ErrorProvider component.

John Saunders
  • 160,644
  • 26
  • 247
  • 397
  • 1
    Regarding Validating/ErrorProvider -- I don't want any validation to occur in the UI. The UI should only validate very simple conditions, such as a field being left blank. Deeper layers should encapsulate the entirety of complex validation so that it is all in one place. – James Aug 14 '09 at 17:36
  • I was thinking in terms of the business layer, in addition to complex rules, also encapsulating things like the format of account numbers. The UI would call such a validation method in the `Validating` event. – John Saunders Aug 14 '09 at 18:02
4

I think you've gotten the wrong impression of the intended message. Here's a great quote I ran across yesterday from the current edition of Visual Studio magazine (Vol 19, No 8).

Either a member fulfills its contract or it throws an excetion. Period. No middle ground. No return codes, no sometimes it works, sometimes it doesn't.

Exceptions should be used with care as they are expensive to create and throw--but they are, however, the .NET framework's way of notifying a client (by that I mean any calling component) of an error.

dotarj
  • 418
  • 2
  • 8
STW
  • 44,917
  • 17
  • 105
  • 161
  • 1
    Please see the quote that I added to the post. "Do not throw exceptions to signal commonly occurring events. Consider using alternate methods to communicate to a caller the occurrence of those events and leave the exception throwing for when something truly out of the ordinary happens." In otherwords, this post is focused on how to handle commonly occuring things that the user may do wrong -- such as misname an item that must adhere to a particular pattern. – James Aug 13 '09 at 14:53
  • 3
    Yup; this is a design decision. Even that (pompous bastard, but occasionally right) "Joel on Software" Joel says "exceptions considered harmful". I agree "either a member fulfills its contract or it throws an exception", but error conditions can be _written into the contract_. – Robert Fraser Aug 13 '09 at 20:41
4

The example you give is of UI validating inputs.

Therefore, a good approach is to separate the validation from the action. WinForms has a built in validation system, but in principle, it works along these lines:

ValidationResult v = ValidateName(string newName);
if (v == ValidationResult.NameOk)
    SetName(newName);
else
    ReportErrorAndAskUserToRetry(...);

In addition, you can apply the validation in the SetName method to ensure that the validity has been checked:

public void SetName(string newName)
{
    if (ValidateName(newName) != ValidationResult.NameOk)
        throw new InvalidOperationException("name has not been correctly validated");

    name = newName;
}

(Note that this may not be the best approach for performance, but in the situation of applying a simple validation check to a UI input, it is unlikely that validating twice will be of any significance. Alternatively, the above check could be done purely as a debug-only assert check to catch any attempt by programmers to call the method without first validating the input. Once you know that all callers are abiding by their contract, there is often no need for a release runtime check at all)

To quote another answer:

Either a member fulfills its contract or it throws an exception. Period.

The thing that this misses out is: What is the contract? It is perfectly reasonable to state in the "contract" that a method returns a status value. e.g. File.Exists() returns a status code, not an exception, because that is its contract.

However, your example is different. In it, you actually do two separate actions: validation and storing. If SetName can either return a status code or set the name, it is trying to do two tasks in one, which means that the caller never knows which behaviour it will exhibit, and has to have special case handling for those cases. However, if you split SetName into separate Validate and Store steps, then the contract for StoreName can be that you pass in valid inputs (as passed by ValidateName), and it throws an exception if this contract is not met. Because each method then does one thing and one thing only, the contract is very clear, and it is obvious when an exception should be thrown.

Jason Williams
  • 56,972
  • 11
  • 108
  • 137
2

I agree part of Henk's suggestion.

Traditionally, "pass/fail" operations were implemented as functions with an integer or bool return type that would specifiy the result of the call. However, some are opposed to this, stating that "A function or method should either perform an action or return a value, but not both." In otherwords, a class memeber that returns a value should not also be a class memeber that changes the state of the object.

I've found the best solution to add a .HasErrors/.IsValid and an .Errors property within the class that generates the errors. The first two properties allow the client class to test wether or not errors exist, and if need be, can also read the .Errors property and report one or all of the errors contained. Each method then must be aware of these properties and manage the error state appropriately. These properties can then be rolled into an IErrorReporting interface that various business rule layer facade classes can incorporate.

H H
  • 263,252
  • 30
  • 330
  • 514
James
  • 1,788
  • 4
  • 22
  • 28
  • IErrorReporting interface seems to be an interesting idea. But in case of multi threaded exeuction, how does a given thread know what are the errors pertaining to this thread? – AllSolutions Mar 08 '13 at 16:18
1

In my opinion, if in doubt, throw exceptions on business rule validation. I know this is somewhat counter-intuitive and I may get flamed for this, but I stand by it because what is routine and what is not depends on the situation, and is a business decision, not a programming one.

For example, if you have business domain classes that are used by a WPF app and a WCF service, invalid input of a field may be routine in a WPF app, but it would be disastrous when the domain objects are used in a WCF situation where you are handling service requests from another application.

I thought long and hard, and came up with this solution. Do the following to the domain classes:

  • Add a property: ThrowsOnBusinessRule. Default should be true to throw exceptions. Set it to false if you don't want to throw it.
  • Add a private Dictionary collection to store exceptions with key as domain property that has business rule violation. (Of course, you can expose this publicly if you want)
  • Add a method: ThrowsBusinessRule(string propertyName, Exception e) to handle tho logic above
  • If you want, you can implement IDataErrorInfo and use the Dictionary collection. Implementation of IDataErrorInfo is trivial given the setup above.
Echiban
  • 849
  • 2
  • 11
  • 21
1

I think I'm close to being convinced that throwing exceptions is actually the best course of action for validation type operations, and especially aggregate type operations.

Take as an example, updating a record in a database. This is an aggregate of many operations that may individually fail.

Eg.

  1. Check there is a record in the DB to update. -- record may not exists
  2. Validate the fields to be updated. -- the new values may not be valid
  3. Update the database record. -- the db may raise additional errors

If we are tying to avoid exceptions, we may instead want to use an object that can hold a success or error state:

public class Result<T> {
  public T Value { get; }
  public string Error { get; }

  public Result(T value) => Value = value;
  public Result(string error) => Error = error;

  public bool HasError() => Error != null;
  public bool Ok() => !HasError();
}

Now we can use this object in some of our helper methods:

public Result<Record> FindRecord(int id) {
  var record = Records.Find(id);
  if (record == null) return new Result<Record>("Record not found");
  return new Result<Record>(record);
}

public Results<bool> RecordIsValid(Record record) {
  var validator = new Validator<Record>(record);
  if (validator.IsValid()) return new Result<bool>(true);
  return new Result<bool>(validator.ErrorMessages);
}

public Result<bool> UpdateRecord(Record record) {
  try {
    Records.Update(record);
    Records.Save();
    return new Result<bool>(true);
  }
  catch (DbUpdateException e) {
    new Result<bool>(e.Message);
  }
}

Now for our aggregate method that ties it all together:

public Result<bool> UpdateRecord(int id, Record record) {
  if (id != record.ID) return new Result<bool>("ID of record cannot be modified");

  var dbRecordResults = FindRecord(id);
  if (dbRecordResults.HasError())
    return new Result<bool>(dbRecordResults.Error);

  var validationResults = RecordIsValid(record);
  if (validationResults.HasError())
    return validationResults;

  var updateResult = UpdateRecord(record);
  return updateResult;
}

Wow! what a mess!

We can take this one step further. We could create subclasses of Result<T> to indicate specific error types:

public class ValidationError : Result<bool> {
  public ValidationError(string validationError) : base(validationError) {}
}

public class RecordNotFound: Result<Record> {
  public RecordNotFound(int id) : base($"Record not found: ID = {id}") {}
}

public class DbUpdateError : Result<bool> {
  public DbUpdateError(DbUpdateException e) : base(e.Message) {}
}

Then we can test for specific error cases:

var result = UpdateRecord(id, record);
if (result is RecordNotFound) return NotFound();
if (result is ValidationError) return UnprocessableEntity(result.Error);
if (result.HasError()) return UnprocessableEntity(result.Error);
return Ok(result.Value);

However, in the above example, result is RecordNotFound will always return false as it is a Result<Record>, whereas UpdateRecord(id, record) return Result<bool>.

Some positives: * It bascially does works * It avoids exceptions * It returns nice messages when things fail * The Result<T> class can be as complex as you need it to be. Eg, Perhaps it could handle an array of error messages in the case of validation error messages. * Subclasses of Result<T> could be used to indicate common errors

The negatives: * There are conversion issues where T may be different. eg. Result<T> and Result<Record> * The methods are now doing multiple things, error handling, and the thing they are supposed to be doing * Its extremely verbose * Aggregate methods, such as UpdateRecord(int, Record) now need to concern them selves with the results of the methods they call.

Now using exceptions...

public class ValidationException : Exception {
  public ValidationException(string message) : base(message) {}
}

public class RecordNotFoundException : Exception  {
  public RecordNotFoundException (int id) : base($"Record not found: ID = {id}") {}
}

public class IdMisMatchException : Exception {
  public IdMisMatchException(string message) : base(message) {}
}

public Record FindRecord(int id) {
  var record = Records.Find(id);
  if (record == null) throw new RecordNotFoundException("Record not found");
  return record;
}

public bool RecordIsValid(Record record) {
  var validator = new Validator<Record>(record);
  if (!validator.IsValid()) throw new ValidationException(validator.ErrorMessages)
  return true;
}

public bool UpdateRecord(Record record) {
  Records.Update(record);
  Records.Save();
  return true;
}

public bool UpdateRecord(int id, Record record) {
  if (id != record.ID) throw new IdMisMatchException("ID of record cannot be modified");

  FindRecord(id);
  RecordIsValid(record);
  UpdateRecord(record);
  return true;
}

Then in the controller action:

try {
  UpdateRecord(id, record)
  return Ok(record);
}
catch (RecordNotFoundException) { return NotFound(); }
// ...

The code is waaaay simpler... Each method either works, or raises a specific exception subclass... There are no checks to see if methods succeeded or failed... There are no type conversions with exception results... You could easily add an application wide exception handler that returns the the correct response and status code based on exception type... There are so many positives to using exceptions for control flow...

I'm not sure what the negatives are... people say that they're like GOTOs... not really sure why that's bad... they also say that the performance is bad... but so what? How does that compare to the DB calls that are being made? I'm not sure if the negatives are really valid reasons.

br3nt
  • 9,017
  • 3
  • 42
  • 63
  • 1
    Thanks, I was looking for this comment, I was trying to build a real-world use case without using exceptions but I stumbled upon the same problems + new ones, the code becomes super verbose with lots of IF statements and some parts of the code may raise exceptions (third-party libs, etc), so I have to catch the exceptions and convert to "Result", logging becomes very painful because you have to put it manually, I'm still trying, but for what I've seen is not sustainable for me. – Thiago Dias Jul 10 '22 at 00:05
  • 1
    @ThiagoDias yeah I agree, it’s about the sustainability of the code. Writing more code to avoid exceptions is a trade off. In this case, it takes more effort to maintain and reason about, and if you leave the code for a while, more effort to read and relearn the logic. – br3nt Jul 10 '22 at 00:13
0

Exceptions are just that: a way to handle exceptional scenarios, scenarios which should not ordinarily happen in your application. Both examples provided are reasonable examples of how to use exceptions correctly. In both instances they are identifying that an action has been invoked which should not be allowed to take place and is exceptional to the normal flow of the application.

The misinterpretation is that the second error, the renaming method, is the only mechanism to detect the renaming error. Exceptions should never be used as a mechanism for passing messages to a user interface. In this case, you would have some logic checking the name specified for the rename is valid somewhere in your UI validation. This validation would make it so that the exception would never be part of normal flow.

Exceptions are there to stop "bad things" happening, they act as last-line-of-defence to your API calls to ensure that errors are stopped and that only legal behaviour can take place. They are programmer-level errors and should only ever indicate one of the following has occured:

  • Something catastrophic and systemic has occurred, such as running out of memory.
  • A programmer has gone and programmed something wrong, be it a bad call to a method or a piece of illegal SQL.

They are not supposed to be the only line of defence against user error. Users require vastly more feedback and care than an exception will ever offer, and will routinely attempt to do things which are outside of the expected flow of your applications.

Paul Turner
  • 38,949
  • 15
  • 102
  • 166
  • 2
    I agree with you. That's the point of this post -- to point out that the second example is NOT the proper way to report the error to the UI. However, I disagree that a solution is to code the business rule right into the UI class itself. The UI class should call down into the business logic layer, the business logic layer performs the validation, and then the business logic layer reports back the status of the validtion to the UI. The UI handles the display of the error. The post is asking for ways of doing exactly that -- communicating errors between layers without exceptions. – James Aug 13 '09 at 15:09
  • An interesting concept, however I personally don't think asking your domain model to perform validation on your UI is a good idea. You will find that the validation requirements of your UI changes depending on the specific design of your UI. In the simplest example, you can have a free-text field or a drop-down-list, both requiring different validation. If the intent of putting your application layers is to separate concerns, asking layers to do UI-specific validation seems a bleeding together of those concerns. I would keep them entirely separate. – Paul Turner Aug 14 '09 at 07:48
  • 1
    I agree, in part. I believe that the UI layer should provide basic validation, but that the validation should be restricted to things like catching empty fields. All domain logic/validation should be removed from the UI. For example, if I have a "Last Name" search field, the UI tests for blank/numeric entry, but a seperate layer will do the search and inform UI layer that the name does not exist. This type of error should not be implemented as an exception. The UI should only make sure the input it sends fits the basic parameter requirements of the lower layer to which it sends the request. – James Aug 14 '09 at 18:43
  • 1
    Can I request your attention to this question: http://stackoverflow.com/questions/15296993/communicating-error-reason-and-data-state-without-using-exceptions – AllSolutions Mar 08 '13 at 16:24
0

Exceptions are a expensive in consume time and resources for the application. and in my opinion should be relegate to those problem that you code can not detect. if a problem is detected you can return to the UI the error simple by creating a simple class with the list of the error response and a code (the code is nor recommended when you handling exception, but in this case you are not handling exception)

for example something like that:

list<Error> errors

public class Error
{
    public int code {get;set;}
    public string description {get;set;}
}

You can return this class inside your object or you can create a tuple and return the values as a second element of the tuple.

If you generate exceptions it is very difficult to know if your application is ok, if you don't generate, any tool that report your exception in your application, give you the idea of how much problem your code has.

Of course also all depend of the size of your application, from the point of view of performance, if you handle 100 or less users in your application you don't note a big difference in use exception or not, but if you are in a web or api service that handle more users that begin to be a big performance problem, maybe it is difficult to see. but if you are in azure for example you can pay a lot of money because the necessity of memory and processor is increased by handle the exceptions.

In this case in my opinion is better avoid exception. a little more complicate code in our side avoid a lot of code execution inside the system.

freedeveloper
  • 3,670
  • 34
  • 39