16 May 2009

The always-valid entity anti-pattern

Jeffrey Palermo wrote an article about the fallacy of the always-valid entity recently in which he highlighted that guarding against an entity becoming invalid by having validation logic in the setters of properties raises more issues than it solves.

Another way of enforcing an always-valid entity is, what I have found elsewhere termed, the "verbose constructor". Having a constructor which requires a value for every property, or every valid combination of properties.

public Car(string make, string model, string serialNumber, 
           decimal engineLitres, int maxPower, decimal urbanMpg, 
           decimal extraUrbanMpg, decimal combinedMpg, 
           int fuelTankCapacity) { ... }

Car myCar = new Car("Audi", "A4", "12345ABCD", 1.8, 120, 
                    28.5, 51.4, 39.8, 65);

The "verbose constructor" is itself a good candidate for an anti-pattern for the following reasons:

  • Modifying the entity means modifying every instantiation of it also
  • Forcing a programmer to set these values when they may not be available will result in garbage data which is useless anyway
  • Derived classes must call this verbose constructor either replicating every parameter in their own constructors or hardcoding values for some parameters (another anti-pattern)
  • Unreadable code, without help from the IDE it's difficult to identify each parameter

The method Jeffrey highlighted in his post has to use exceptions to catch validation errors, which forces a programmer to use another anti-pattern; exception handling. Where programming logic is implemented by the throwing and catching of exceptions. Any call to a property setter of the entity must be wrapped in a try...catch... block.

The always-valid entity, however it's implemented, seems to lead to a minefield of bad code. The thinking behind the need for this blurs the line between two important distinctions:

  • not setting a value vs. setting it to something incorrect
  • what makes an object a valid type vs. what makes an object valid for a particular use

I touched on the first of these before; where it may be the case that a programmer doesn't yet have a value for a particular property available yet. So long as we ensure the entity doesn't leak out in this invalid state we're okay.

Which leads on to the second item; we should guard against a value being set where that value violates the type e.g. setting Day to 32 for a DateTime, something bad has occurred and this is a real exception case. However, this is a much lower-level problem than enforcing business rules, for example where a credit card applicant's date of birth must be at least 18 years earlier than the date of the application. It could be the case that one company actually requires applicants to be 21 so hardcoding that type of validation into the class is not only restrictive but enforcing this by throwing exceptions is very inefficient.

The always-valid entity just serves to be very restrictive and limits the entity's uses in its current form. Validating from outside means that its easy to skip validation or make it more stringent when needed. Also, if implemented along the lines Jeffrey describes in his post, it means mechanisms have a common way of validating objects without needing specific knowledge of the particular object or the current validation being applied which makes for more robust code.

4 comments:

Andrew said...

Did you read the comments at the end of Jeffery's post? There are some compelling arguments in favour of "always-valid".

andy said...

It's better to have to refactor the parameters for a constructor at instance creation than to allow a developer not to set a property that is required for an entity to be valid, or to fail to alter each place where it should be set. The compiler is on your side and tools like Resharper make this painless.

Constructor parameters usually reflect the minimum set of properties that are required for the initial state of the entity instance. Your car is perhaps burdened with mandatory information that should be optional?

Exception handling should be sparse. Try/catch around every call to an entity would indeed be an anti-pattern, however a better approach is to catch the exception once at an appropriate level (in this case the UI). Your validation framework is going to collate all of the violated rules and throw one exception.

Derek Fowler said...

@Andrew
I did read them, and still am.

Derek Fowler said...

@andy
I'd argue that this is incorrect because a. modifying a constructor goes against the OO principal of extend rather than modify and b. you may not own the instantiation code to be able to take advantage of the compiler or Resharper catching the change.

I'm not saying that parameterised constructors are bad, they are essential, especially for behavioural classes that just wont work without some outside object being passed in. Here I'm looking at the case where you're forced to set values for *every* property of the entity. Where the business rules surrounding what makes the entity valid are hardcoded into the structure of the class.

Exception handling should not need to be used to implement normal business logic at all, it should cater for exceptional cases only i.e. the unforseen, not the expected "user typing in the wrong thing" case. Also by implementing validation using exceptions in properties you're only going to be able to identify one invalid value at a time, no collation can occur unless you use multiple try/catches. I'd also argue that allowing an exception to bubble all the way out of your app tier to the UI as a "feature" of your program is bad practice.

I think your presentation tier code should prepare itself an entity with the values from the user input. It should then apply validation to the entity to check it's all good, notifying the user of any problems if they arise. Only once this has passed should it attempt to save the entity and at this point if the entity is still invalid we have an exception case.