views:

50

answers:

3

The following example is clearly fictional but it resumes how validation is done on a code base I'm working with.

TypeA has two methods, with the following signatures:

public void FirstMethod(TypeB param)
public ValidationResult TryFirstMethod(TypeB param)

When FirstMethod is called it needs to perform validation on the parameter.
Hence it calls TryFirstMethod to retrieve an object representing the validation's result.
If the instance of ValidationResult says that everything's OK the execution continues otherwise an exception is thrown.

The utility of TryFirstMethod is that the caller, an hypothetical TypeC, can execute this method and check if the actual method would throw. By examining a property on ValidationResult.
Also, the instance of ValidationResult contains information on why the input was wrong, how to fix it and so on.
This justifies the need for this type instead of just, say, using a boolean.

In practice this works fairly well, it's quite easy to validate data and to return localized error messages to the user.

The only issue is that since certain checks are quite complex performing them two times becomes a little expensive.
They are initially done by the caller to make sure that the actual method won't throw.
And then again by the method in question to check that the input is valid.

I can't find a clean way to avoid having to perform the checks two times.
To make things more complex there is the fact that the solution should work with standard methods, constructors and also when TypeB is a "primitive" type like string or int.

A: 

Since the first method is a void, why not just call it to begin with and have it return a ValidationResult?

Can you give us any context on why the first one and second one have the same code that -has- to be run twice?

You could simply have the first one fail and pass the error messages back to the user in that route...

Aequitarum Custos
The void is just to keep the example simple. There often is a return type. I don't use exceptions exclusively for the reasons explained in my comment to akmad's answer.
RobSullivan
A: 

The cleanest solution in my eyes would be a third method:

public void FirstMethod(TypeB param, ValidationResult alreadyPerformedResult)

So the user can make it's own check, call the desired function with the result of the check. So you can skip the check and just test if the results of the test also meets your criterias too.

Maybe you can add something to your ValidationResult (e.g. function name as string in simplest scenario) so that you're sure, that the given result is really from this method and not from another called TryMethod().

Oliver
+1  A: 

It sounds like your core logic is something like this:

  1. Validate TypeB (call TryFirstMethod).
  2. If not valid, display error message.
  3. If valid (and, presumably, other members are valid as well) then to perform operation on the data (as part of FirstMethod?).
  4. The data operation (FirstMethod) re-validates the input.

If this is (somewhat) correct, you should be able to change your design into something like:

  1. Call FirstMethod.
  2. Validate data.
  3. If invalid, throw validation exception.
  4. If valid, perform data operation.

Your UI would then call FirstMethod and expect that the operation was successful unless an exception was thrown, in which case you'd display your validation messages as you do currently. With this design you only perform the validation a single time.

akmad
Yes, you understood perfectly. The fact is that exceptions are expensive themselves, and data coming from the outside world can be invalid quite often. Again, a performance issue. Also, I prefer to keep exceptions as light as possible.
RobSullivan
Exceptions are expensive compared to what? If the validation procedure is, as you state, complex then the cost of throwing an exception is likely negligible in comparison. If the cost of throwing an exception does meaningfully affect then just return a ValidationResult as you are in TryFirstMethod.
akmad