views:

284

answers:

5

Suppose you have a subsystem that does some kind of work. It could be anything. Obviously, at the entry point(s) to this subsystem there will be certain restrictions on the input. Suppose this subsystem is primarily called by a GUI. The subsystem needs to check all the input it recieves to make sure it's valid. We wouldn't want to FireTheMissles() if there was invalid input. The UI is also interested in the validation though, because it needs to report what went wrong. Maybe the user forgot to specify a target or targetted the missles at the launchpad itself. Of course, you can just return a null value or throw an exception, but that doesn't tell the user SPECIFICALLY what went wrong (unless, of course, you write a separate exception class for each error, which I'm fine with if that's the best practice).

Of course, even with exceptions, you have a problem. The user might want to know if input is valid BEFORE clicking the "Fire Missles!" button. You could write a separate validation function (of course IsValid() doesn't really help much because it doesn't tell you what went wrong), but then you'll be calling it from the button click handler and again from the FireTheMissles() function (I really don't know how this changed from a vague subsystem to a missle-firing program). Certainly, this isn't the end of the world, but it seems silly to call the same validation function twice in a row without anything having changed, especially if this validation function requires, say, computing the hash of a 1gb file.

If the preconditions of the function are clear, the GUI can do its own input validation, but then we're just duplicating the input validation logic, and a change in one might not be reflected in the other. Sure, we may add a check to the GUI to make sure the missle target is not within an allied nation, but then if we forget to copy it to the FireTheMissles() routine, we'll accidentally blow up our allies when we switch to a console interface.

So, in short, how do you achieve the following:

  1. Input validation that tells you not just that something went wrong, but what specifically went wrong.
  2. The ability to run this input validation without calling the function which relies on it.
  3. No double validation.
  4. No duplicate code.

Also, and I just thought of this, but error messages should not be written in the FireTheMissles() method. The GUI is responsible for picking appropriate error messages, not the code the GUI is calling.

+2  A: 

This is what Model-View-Controller is all about.

You build up a model (a launch which is composed of coordinates, missile types and number of missiles) and the model has a validate method which returns a list of errors/warnings. When you update the model (on key-up, <ENTER>, button-press) you call the validate method and show the user any warnings, errors, etc. (Eclipse has a little area just under the tools bar in a dialog that does this, you might want to look at that.)

When the model is valid, you activate the launch missiles button so that the user knows that they can launch the missiles. If you have an update event that is called particularly frequently or a part of the validation that is particularly costly, you can have a validate_light method on the model that you use for validating only the parts that are easy to do.

When you switch to a console based UI you build up the model from the command line arguments, call the same validate method (and report errors to stderr) and then launch the missiles.

Aaron Maenpaa
MVC is about a heck of a lot more than input validation. When it gets down to the input validation itself, is what you're suggesting any different than what Binary Worrier is suggesting? If so, how? If not, then is it really necessary to do MVC in order to acheive this error handling method? (Not to say that the final application necessarily WON'T be MVC, but I'm not even thinking about the GUI yet, except insofar as knowing what it will require from my missle firing subsystem.)
Daniel Straight
+1 In short: The model defines what is valid for any part of it and anyone has to make sure they heed these constraints.
Aaron Digulla
A: 

I'd suggest an input validation class, which takes the input type (an enumeration) in its' constructor, and provides a public IsValid method.

The IsValid method should return a boolean TRUE for valid and FALSE for invalid. It should also have an OUT parameter that takes a string and assigns a status message to that string. The caller will be free to ignore that message if it wants to, or report it up to the GUI if that's appropriate for the context.

So, in pseudocode (forgive the Delphi-like syntax, but it should be readable to anybody):

//different types of data we might want to validate
TValidationType = (vtMissileLaunchCodes, vtFirstName, 
  vtLastName, vtSSN);  

TInputValidator = class
public
  //call the constructor with the validation type
  constructor Create(ValidationType: TValidationType);

  //this should probably be ABSTRACT, implemented by descendants
  //if you took that approach, then you'd have 1 descendant class
  //for each validation type, instead of an enumeration
  function IsValid(InputData: string; var msg: string): boolean;

And then to use it, you'd do something like this:

procedure ValidateForm;
var
  validator: TInputValidator;
begin
  validator := TInputValidator.Create(vtSSN);
  if validator.IsValid(edtSSN.Text,labelErrorMsg.Text) then 
    SaveData;  //it's valid, so save it!
  //if it wasn't valid, then the error msg is in the GUI in "labelErrorMsg".
end;
JosephStyons
A: 

Each piece of data has its own meta data (type, format, unit, mask, range etc.). Unfortunately this is not always specified.

The GUI controlls need to check the input with the metadata and give warnings/errors if the data is invalid.

Example: a number has a range. The range is provided by the metadata, but the range check is provided by the control.

Gamecat
+3  A: 

"The subsystem needs to check all the input it receives to make sure it's valid"

Think of the inputs not so much as a list of arguments, but as a message, it gets easier after that.

The message class has an IsValid member function, it remembers if IsValid was called and what the result was. It also remembers its state, if the state changes then it needs to be re validated. This message class also keeps a list of validation errors.

Now, the UI builds a TargetMissiles message, and the UI can validate it, or pass it directly to the MissileFiring subsystem, it checks to see if the message was validated, if not it validates it, and proceeds / fails depending. The UI gets the message back, with the list of validations already populated.

The messages with their validation sit in a separate library. No code is duplicated.

This sound OK?

Binary Worrier
+1  A: 

Double the validation. In many case the validation is trebled (FKs and not null fields in the DB for example). Depending on your platform it may be possible to code the validation rules once. For example your front end and backend code could share C# business classes. Alternatively you could store the validation rule as metadata that both the backend and front end can access an apply.

In reality the fact that you need different responses to a validation problem (for example the Fire Missile button shouldn't even be enabled until the other inputs are valid) there will be different code associated with the same rule.

AnthonyWJones
What about the problem of adding a validation rule in one place but not others?
Daniel Straight
If you think that is serious enough problem then you need to use a system the allows you to define the rule only once but quite often such systems have some significant draws backs. You should consider that the backend MUST validate and the GUI SHOULD validate and in a very user friendly way, if you miss something in the GUI you get an ugly message but things are still validated.
AnthonyWJones
I can definitely see the value of this in simple cases, but I think it breaks down eventually as the number of fields being validated and the complexity of the validation logic increases. Repeating 50 lines of validation code is one thing. Repeating 500 is another.
Daniel Straight