Is there a best practice way to validate user input?
This is a simple requirement but sometimes being debated. This is my "current" approach how to deal with validation. I have not yet used this approach, and this is just a concept. This approach need to be developed more
First, create a custom validation attributes
public class ValidationAttribute : Attribute{
public type RuleType{get;set;}
public string Rule{get;set;}
public string[] RuleValue{get;set;}
}
Second, create a custom error handler / message
public class ValidationResult{
public bool IsSuccess{get;set;};
public string[] ErrorMessages{get;set;};
}
Then create a validator
public class RuleValidator{
public ValidationResult Validate(object o){
ValidationResult result = new ValidationResult();
List<string> validationErrors = new List<string>();
PropertyInfo[] properties = o.GetType().GetProperties();
foreach(PropertyInfo prop in properties){
// validate here
// if error occur{
validationErrors.Add(string.Format("ErrorMessage at {0}", prop.Name));
//}
}
result.ErrorMessages = validationErrors.ToArray();
}
}
To use it, then you can do like this:
public class Person{
[ValidationAttribute(typeof(string), "Required", "true")]
public string Name{get;set;}
[ValidationAttribute(typeof(int), "Min", "1")]
public int Age{get;set;}
}
To call the validator
public void ValidatePerson(Person person){
RuleValidator validator = new RuleValidator();
ValidationResult result = validator.Validate(person);
// generate the error message here, use result.ErrorMessages as source
}
What is the advantage:
- You can use in any application platform (Winforms, Asp.Net, WCF, etc)
- You can set the rule in attribute-level
- It can do automated validation
- This approach can be used with DependencyInjection with custom validators to separate validation logics
The disadvantage:
- Hard to create the validators
- If not handled well, the validators can become very large in number
- Bad performance due to use of reflection
I was planning to create a ErrorMessages class that contains a list where I can put all errorMessages. Every time an invalid input is given, a new message is added to the errorMessages list. So if user click's 'create' button, all messages in the list are shown. Is this a good way of handling things?
Subjectively, I think it would be better to provide instant feedback that the value the user entered is invalid. That way, they can immediately go back and fix it.
I mean, think about it. The approach you propose would literally give them a giant list of problems at the end, which is not very user-friendly. Besides, how are they going to remember all of those problems to be able to go back and fix them one at a time? (Hint: they're not.)
Instead, I recommend using the ErrorProvider
class to display any errors right next to the appropriate control. I talked a little bit more about this approach in my answer here and here.
Of course, you'll still need to make sure upon final submission (clicking the OK/Submit button) that all the input is valid, but then that's just a simple case of checking for the presence of any errors.
I could easily do this in the Form class. But I remember some best practice way of validating the input in the set properties.
Yes, the idea here is encapsulation. The Form class should only know about form stuff. It shouldn't be required to know what kind of input is/is not valid for all of your different controls.
Instead, this validation logic should be placed elsewhere, such as in a class that stores your data. That class would expose public properties to get and set the data, and inside of the setter method, it would verify the data.
That means that all your Form has to do is call a setter method on your data class. The Form needs to know nothing about how to validate the data, or even what the data means, because the data class handles all of that.
That should not happen, no instance of the class may be created unless input is valid.
If this is indeed the case, you will need to provide a constructor for the class that accepts as parameters all of the data it needs. The body of the constructor will then validate the specified data and throw an exception if any of it is invalid. The exception will prevent the class from being created, ensuring that no instance of a class that contains invalid data ever exists.
Such a class would probably not have setter methods at all—only getters.
However, this is kind of an unusual requirement in the world of C# (however common it may be in C++). Generally, placing your validation code inside of the setters works just fine.
My properties have some private setters. So they only get set in the constructor of my data class. Problem is now that this seems to make my validation not eassy
Why would that change anything? You still handle the validation inside of the private setters. If validation fails, you throw an exception. Because the constructor doesn't handle the exception, it continues bubbling up out of that method to the code that attempted to instantiate the object. If that code wants to handle the exception (e.g., to display an error message to the user), it can do so.
Granted, throwing an exception in the case of invalid input is not necessarily a "best practice". The reason is that exceptions should generally be reserved for unexpected conditions, and users screwing up and providing you with invalid data is, well, to be expected. However:
- This is the only option you have for data validation inside of a constructor, because constructors can't return values.
- The cost of exception handling is basically negligible in UI code since modern computers can process exceptions faster than users can perceive on-screen changes.
See the ErrorProvider
class (documentation here). It provides a set of standard visual indicators that can be attached to most of the standard WinForms controls.