tags:

views:

213

answers:

11

Let's say I have a model Address which get entered from UI. I have to validate that the address never gets saved into a system in an incomplete state (whatever that might mean to the business).

When the user enters an address in the UI, it gets serialized into an Address object. So I was wondering: where does a method like "isComplete" belong? The validator or the model?

If I put that method in the address model and call address.isComplete() in the validator before saving, it implies that incomplete address is a valid system state; if I do the completeness check in the validator, it feels like the validator has too much knowledge of the address internals.

I'm wondering, what general convention do others follow?

EDIT : As someone mentioned below , in the example above address object is reused across multiple systems and "full" object in one system may not mean "full" object in another system . So there is no consistent way enforcing "An object should never be constructed in a invalid state" globally because invalid state is context dependent.

+1  A: 

It sounds like you need an AddressFactory, where you pass in the data for an address, and it'll either give you a valid Address, or an error (exception/whatever). You're right to identify that it's not ideal to create an invalid Address.

If you do this, you should determine who can create Address objects and if they can be created without using the AddressFactory. If so, then the Address object itself needs to protect against invalid inputs, and the argument for the Address object itself having the validity checking becomes stronger.

Brian Agnew
+1  A: 

Validator, for two reasons.

First, there's little to no inherent behavior in an address -- it's just data.

Second, because validation may change depending on the use of the address. You might have a rule that prohibits PO boxes when shipping large items, but not care for billing address. While for billing address, you might want to make more stringent checks so that you don't have to pay your payments gateway when someone puts in a bogus address.


Responding to the comments: does the data differ between BillingAddress and ShippingAddress? Or do the rules change? And are the rules cast in stone, or will they perhaps change in the future (ie, UKBillingAddress, USBillingAddress)?

I agree that there are cases where it's useful to have the type system enforce data rules (measurements are a good example; Google for the Mars probe that used meters in one place and feet in anither). However, I suspect that addresses are not one of those cases.

kdgregory
If we want domain to be self contained and rich , shoulnt we put those methods on address as well , in the above case should there be a BillingAddress and ShippingAddress classes with their own definition of "completeness" ?
Surya
I agree, there should be a BillingAddress and ShippingAddress as separate classes of data. You shouldn't never have an invalid BillingAddress nor an invalid ShippingAddress.
Triynko
A: 

If you don't want your validator to know about what it's validating, you can either use an interface or an inversion of control pattern. On the interface side, it's easy ...

interface IValidateable  // or whatever :)
{
   bool IsValid { get; }
}

That way, the validator only knows that it needs to ask an object if it is valid knowing nothing about the internal state of that object.

On the IoC side, that is more complicated. You would hand the validator a particular method signature that it would call on an object (i.e. to validate me, call this). But, that's a more difficult solution.

JP Alioto
A: 

You could look at a Composite. Have a base Validator, then sub-classed to a specific AddressValidator. Now it makes sense for the validator to have knowledge of how addresses work, but not for it to worry about how any other items validate.

Then, group you many validators into ValidatorCollection, and call it's .Validate() method and it calls the same on all children.

CodexArcanum
A: 

Is isComplete() a check or a command?

If it is a check function (if (isComplete()) {}) then it belongs in the validator. Since you have a UI and a model I'll assume you have a controller that coordinates between the two. In that case just call your check function from the validator before calling your save routine which will mark your model as Complete.

If isComplete() is a command that marks your data as complete then I would think you should change that part of the design. Too messy for my tastes.

Robert Kozak
A: 

IMO address model should have the information whether the address is complete or incomplete.

lets say, later if the business logic changes and now incomplete means at least 5 characters. Then why should validator worry about this logic change?.

I feel, objects should hold their data and state as well.

aJ
A: 

I think that everything you've suggested is relevant. There are a couple ways you can go about this:

Make validator an interface:

This could be done like

Validator addressValidator = new AddressValidator();
bool valid = addressValidator.validate(map);
if (valid) {
    Address address = addressValidator

This way you recognize that the addressValidator is coupled to the Address. Alternately, you could do this using generics

Put the isComplete in Address:

It is a good oop heuristic to say that all objects should construct with valid state, but it just doesn't always hold. Remember, oop maximizes the compile-time information you can take advantage of, i.e. force as many errors to occur at compile rather than run time. User Interfaces will thwart this at all turns. You kind of have to violate this somewhere. (I think the J2EE convention is to construct objects without parameters and call a hundred setters, for example.)

Split Address int two objects:

If you want to be technical about valid state, then you really have two different standards: the state returned by your UI and the state needed by your model. Why not make the difference explicit? For example, if my UI has 12 fields available, I could have a UIAddress object that has 12 String fields. I'm in control of this: regardless of what the user does, I can make sure the UI returns 12 fields. You can then create an instance of UIAddress and call isComplete() on that:

UIAddress uiAddress = uiForm.getAddress();
if (uiAddress.isComplete()) {
    Address modelAddress = uiAddress.cleanedData();
}

It's all going to depend on your intended design, but there really is a lot of flexibility for you. IMHO, the "get-it-done" way is to put isComplete in the Address.

David Berger
A: 

IMHO.. you'll need 2 levels of validation.

  • Client/UI level: Find rules that have a low affinity for change but are easy to change and slot them into the UI. e.g. this field's content 'has to be numeric' or this field's content has to 'match this pattern' or 'is required'. Declarative rules live here..giving the user immediate feedback.
  • Model level: Move everything else into model/business rules. Anything that can change should go in here. You cannot do away with model validation because I may do this via code bypassing the GUI
Address a = new Address();
// set fields as I deem fit
a.Save();

TO summarize: Try to flag problems and weed out bad input as far as possible without hitting the model. The model is your last line of defence and ensures that no bad input gets past it.

Gishu
A: 

if this is a data-entry application where the user must complete the entire form/address before anything is persisted, then the Validator should own the isComplete method and the model will never have to deal with anything other than complete addresses

BUT, if the user may need to save partial results and continue them later (e.g. if the form containing the address is just one step in a 'wizard' sequence that can be interrupted/continued) then isComplete belongs to the model, and 'incomplete' is a valid state for an address.

Alternately, the isComplete method could belong to the Validator (whatever that means in your framework!) but its complete/incomplete state becomes part of the address data.

Steven A. Lowe
+1  A: 

I create a data class for each kind of data I use. I do not use "string" and "int" types, except within those data classes. My data classes have meaningful names, and are used in contexts where the class's particular constraints are meaningful.

For example, if you have a "score" field that is only meaningful with values between 1 and 100, then you have NO BUSINESS storing it in an "int" class, whose range is -2,147,483,648 to 2,147,483,647. If you do it, have fun trying to validate it redundantly everywhere you pass it, and have fun figuring out when and where you need to validate it, and have fun when you forget a spot and let invalid data sneak into your system, and have fun trying to explain to me how a bug that's not the programmer's fault is possible in software written for deterministic digital hardware.

When/Where to Validate:

If you are accepting a score from a text field in a UI, perform preliminary validation on it at the point of entry, giving error messages appropriate for that particular user interface. The preliminary validation can be as complex as you need it to be to provide meaningful feedback to the user.

Once you've performed the initial validation, stuff the data in an appropriate meaningful data class like "Score", where the constructor will perform the final/authoritative validation. This authoritative validation should be as simple and efficient as possible, but absolutely sufficient for guaranteeing the value is valid. It's a boolean validation; the data is either valid or an exception is thrown appropriate for a programmer to read. If your data class's validation code is, for example, simply a regular expression match, consider including the data class name, the regular expression string, and the data itself in the error message. All basic data classes should be immutable, like the string class itself; once constructed, it is guaranteed to be valid and remain so.

Your preliminary validation code may or may not leverage the data class's validation code. It depends on how detailed the data you're collecting is, and how much feedback you want to send to the UI. Similarly, more complex classes you create, which contain data classes as fields, should not be constructable unless they are valid. So you may need to write code that validates or attempts to construct each individual data field, catch the low-level construction errors, and handle them by providing more appropriate feedback to the UI. Sometimes, this feedback can involve coloring a field red and displaying an error message next to it. So as you can see, there are two kinds of validation, and one is clearly more complex out of necessity.

Triynko
A: 
Robert Tuck