2

Is it a bad practice to write methods which return unthrown exceptions for the purpose of input validation? The Validate method will return null if the input is valid, or return the exception that will be thrown should the input actually be submitted.

public Exception Validate(object input)
{
    if (!SomeParametersMatch(input))
        return new SomeException("Message...");
    if (!SomeOtherParametersMatch(input))
        return new SomeOtherException("Another message...");

    // More cases here...

    return null;
}

This way, you can use the same function for validating input, displaying responses to the user, and throwing exceptions in the code:

public void Submit(object input)
{
    Exception ex = Validate(input);
    if (ex != null) throw ex;

    // Do whatever action here...
}

For example, if you're using the functions to mark spaces valid to click, you can call Validate for each space, marking them valid if the return value isn't null. Then Submit is only called once the user actually clicks on a space and the choice is finalized. This removes code duplication when you need to be able to ensure an input will be valid should you choose it.

I could make Validate return a void and simply throw the exceptions, but since catching thrown exceptions is the most part of exception throwing, and Validate will be run on many more invalid inputs than valid inputs, it seemed like a waste. If Validate were only used when the user actually submitted data, I would have no problem using a try/catch block. But since it's being used to filter data presented to the user, throwing an exception the majority of the time, only to catch and discard it, seems ecessively wasteful.

Community
  • 1
  • 1
dlras2
  • 8,416
  • 7
  • 51
  • 90
  • You code in `// Do whatever action here...` will not be executed if you need to throw as you are first throwing the exception. – Guillaume Mar 16 '12 at 05:29
  • Only if the input is not valid. If it is valid, `Validate` will return null, and the `Submit` function will run. – dlras2 Mar 16 '12 at 05:29
  • To me it looks strange; you seem to replace a `throw` with a `return`, basically. Is this really worth the effort? Drawback is that you have to remember to throw outside `Validate()`. Throwing inside `Validate()` helps you in case you forgot. – Uwe Keim Mar 16 '12 at 05:35

2 Answers2

1

Where does it say try-catch blocks are expensive? What benefit does returning an exception have other throwing it?

public void Validate(object input)
{
    if (!SomeParametersMatch(input))
        throw new Exception("Message...");
    if (!SomeOtherParametersMatch(input))
        throw new Exception("Another message...");

    // More cases here...

}

public void Submit(object input)
{
    Validate(input);

    // Do whatever action here...
}

This code is more readable and is less confusing. This is exactly how exceptions are meant to be used. Whatever expense there is to using a try-catch block is negligable.

Hand-E-Food
  • 12,368
  • 8
  • 45
  • 80
  • I am expecting `Validate` to be run on many many invalid inputs, in order to weed them out before the user can submit them. `try` has a negligible cost, but catching a thrown exception requires a fair amount of work. See http://stackoverflow.com/questions/867017/performance-cost-of-try-in-c-sharp – dlras2 Mar 16 '12 at 05:36
  • Unless you're simultaneously validating millions of inputs, the user is not going to experience a percievable delay in catching exceptions. Besides that, you're throwing the exception in the `Submit` method anyway. – Hand-E-Food Mar 16 '12 at 05:52
  • I'm validating spaces on a game board inside of a game loop. And yes, it's thrown from `Submit` but that's only called once per turn. (Assuming the input is valid.) – dlras2 Mar 16 '12 at 06:03
  • If `Validate` throws an exception, there will still only be one exception thrown per call to `Submit`. Unless `Submit` catches the exception, the thrown exception will break out of the `Validate` and `Submit` methods, preventing further validation from occuring. Try using the code above and submit a test case with many invalid inputs. If `Submit` is inside a try-catch, you'll only ever catch the first exception thrown by any method call. – Hand-E-Food Mar 16 '12 at 06:11
  • My response stands: it's bad practice. The best thing in your situation is to try both options and time them. – Hand-E-Food Mar 18 '12 at 22:06
0

Validation libraries like DataAnnotations is a workaround and not the proper way to handle validation of user input. The best way is to validate values is from within each property that is set.

DataAnnotations and corresponding validation libraries have the side effect that all objects are in an inconsistent state before the object is validated. Validation inside property setters do not have the same problem.

However, using such libraries are the best approach sometimes. But not every time. Make sure that it's the proper design choice.

Having that said, I would build up a list of validation errors inside the validation method and then throw an exception will all errors included.

jgauffin
  • 99,844
  • 45
  • 235
  • 372