Input validation for a user

Question

I have a small Person Domain Model as follows:

public class Person
{
    public Person()
    {
        FirstName = "John";
        LastName = "Doe";
        BirthDate = DateTime.Now.AddYears(-18);
    }

    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public Guid PersonId { get; set; }

    [MaxLength(25, ErrorMessage = "The First Name cannot be more than 25 characters in length.")]
    [MinLength(3, ErrorMessage = "The First Name must be at least three characters.")]
    [Required(ErrorMessage = "The First Name is required.", AllowEmptyStrings = false)]
    [DisplayName("First Name")]
    public string FirstName { get; set; }

    [MaxLength(25, ErrorMessage = "The Last Name cannot be more than 25 characters in length.")]
    [MinLength(3, ErrorMessage = "The Last Name must be at least three characters  in length.")]
    [Required(ErrorMessage = "The Last Name is required.", AllowEmptyStrings = false)]
    [DisplayName("Last Name")]
    public string LastName { get; set; }

    [DisplayName("Full Name")]
    public string FullName => string.Format("{0} {1}", FirstName, LastName);

    [Date(ErrorMessage = "Age must be between 18 and 65 years of age")]
    [DisplayName("Birth Date")]
    [DataType(DataType.Date,ErrorMessage = "This is not a valid date.")]
    [Required(ErrorMessage = "The Birth Date is Required")]
    public DateTime BirthDate { get; set; }

    [RegularExpression(@"^((?!000)(?!666)(?:[0-6]\d{2}|7[0-2][0-9]|73[0-3]|7[5-6][0-9]|77[0-2]))-((?!00)\d{2})-((?!0000)\d{4})$",
        ErrorMessage = "The Social Security Number must be in the pattern xxx-xx-xxx and be a valid US SSN.")]
    [DisplayName("Social Security #")]
    [Required(ErrorMessage = "The Social Security Number is Required.",AllowEmptyStrings = false)]
    public string SocialSecurityNumber { get; set; }

    public override string ToString()
    {
        return FullName;
    }
}

If I create a new MVC web site and scaffold my controller, all my validating attributes just work on the create new and edit screens.

However, if I needed a WinForm to do the same thing, I would need to "roll my own". I did just that.

My WinForm code-behind in all its glory:

public partial class AddPersonForm : Form
{
    private readonly DemoContext _context;

    public AddPersonForm()
    {
        _context = new DemoContext();
        InitializeComponent();
    }

    protected override void OnLoad(EventArgs e)
    {
        _context.People.Load();

        personBindingSource.DataSource = _context.People.Local.ToBindingList();

        personBindingSource.RaiseListChangedEvents = true;

        FirstNameTextBox.Validating += Input_Validating;
        LastNameTextBox.Validating += Input_Validating;
        BirthDateDateTimePicker.Validating += Input_Validating;
        SocialSecurityNumberTextBox.Validating += Input_Validating;

        personBindingSource.AddNew();

        base.OnLoad(e);
    }

    private void personBindingNavigatorSaveItem_Click(object sender, EventArgs e)
    {
        personBindingSource.EndEdit();
        int changes = _context.SaveChanges();
        Debug.WriteLine("# of changes: " + changes);
    }

    private void Input_Validating(object sender, CancelEventArgs e)
    {
        // GetValidationErrors cannot see any errors unless the following line is present
        personBindingSource.EndEdit();

        IEnumerable<DbEntityValidationResult> results = _context.GetValidationErrors();
        foreach (DbEntityValidationResult result in results)
        {
            // Stops at the first error and also cancels the endedit.
            foreach (DbValidationError error in result.ValidationErrors)
            {
                Debug.WriteLine("Property: " + error.PropertyName);

                if (error.PropertyName == "FirstName")
                {
                    errorProvider1.SetError(FirstNameTextBox, error.ErrorMessage);
                    //e.Cancel = true;
                    return;
                }

                if (error.PropertyName == "LastName")
                {
                    errorProvider1.SetError(LastNameTextBox, error.ErrorMessage);
                    //e.Cancel = true;
                    return;
                }

                if (error.PropertyName == "BirthDate")
                {
                    errorProvider1.SetError(BirthDateDateTimePicker, error.ErrorMessage);
                    //e.Cancel = true;
                    return;
                }

                if (error.PropertyName == "SocialSecurityNumber")
                {
                    errorProvider1.SetError(SocialSecurityNumberTextBox, error.ErrorMessage);
                    //e.Cancel = true;
                    return;
                }
            }
        }

        errorProvider1.Clear();
    }
}

It may look bad, but it does work. How do I make my method Input_Validating() more efficient so I don't have to write a new block for every input control on the form?

Answer 1

You could merge your two foreach loops using SelectMany in LINQ.

So instead of :

foreach (DbEntityValidationResult result in results)
    foreach (DbValidationError error in result.ValidationErrors)

You'd have

foreach (DbValidationError error in results.SelectMany(r => r.ValidationErrors))

Now, that's up to you to find it more readable or not, I just think it removes some lines of codes and is more readable to someone who knows LINQ.

But wait, there's more! Considering you return at the first your find, using FirstOrDefault might be a good choice :

var result =  results.FirstOrDefault(r => r.ValidationErrors.Any());
var error = result.ValidationErrors.First();

This is pretty simple, we'll get the first result that has at least one validation error. And then we get that error.

It might not be the most performing algorithm, but it is at least equal to what you're doing at the moment, and that's not bad at all.

What's the next step? We'll remove those if statements. How? Using a Dictionary<string,Control>! You have a binding between some property names and some other values, that seems like a perfect fit for the dictionary.

Note : I'm not entirely sure Control is the good class for the value of the dictionary, but it depends on what's done in the SetError method, which I don't see at the moment.

It would look like this :

var errorMap = new Dictionary<string,Control>();
errorMap.Add("FirstName", FirstNameTextBox);
errorMap.Add("LastName", LastNameTextBox);
//etc..

What does the final code look like now?

// The dictionary has been initialized in the constructor of this class, or    
// somewhere like that. It has to be done once per instance!

private void Input_Validating(object sender, CancelEventArgs e)
{
    // GetValidationErrors cannot see any errors unless the following line is present
    personBindingSource.EndEdit();

    IEnumerable<DbEntityValidationResult> results = _context.GetValidationErrors();

    //Get the first validatio result that has an error, if any.
    DbEntityValidationResult result =  results.FirstOrDefault(r => r.ValidationErrors.Any());

    if(result == null)
    {
        errorProvider1.Clear();
        return;
    }

    //Gets the first error of the validation result.
    DbValidationError error = result.ValidationErrors.First();

    errorProvider1.SetError(errorMap[error.PropertyName], error.ErrorMessage);
}

Now, I don't check if the dictionary contains the property name before trying to get it via the indexer. I don't think it's necessary in your case, since you have full control over what property is used and to which control it is bound, that shouldn't change during runtime.

What if we wanted this code to be even nicer. We could extract a method to get the first error in the validation results!

public DbValidationError GetFirstError(IEnumerable<DbEntityValidationResult> validationResults)
{
    //Get the first validation result that has an error, if any.
    DbEntityValidationResult result =  results.FirstOrDefault(r => r.ValidationErrors.Any());

    if(result == null) { return null; }

    return result.ValidationErrors.First();
}

Note that if you use C#6, you could use the null propagation operator ?. like this :

    //Get the first validation result that has an error, if any.
    DbEntityValidationResult result =  results.FirstOrDefault(r => r.ValidationErrors.Any());

    return result?.ValidationErrors.First();

Then our code would look like this :

private void Input_Validating(object sender, CancelEventArgs e)
{
    // GetValidationErrors cannot see any errors unless the following line is present
    personBindingSource.EndEdit();

    IEnumerable<DbEntityValidationResult> results = _context.GetValidationErrors();

    var error = GetFirstError(results);

    if(error == null)
    {
        errorProvider1.Clear();
    }
    else
    {
       errorProvider1.SetError(errorMap[error.PropertyName], error.ErrorMessage);
    }
}

Answer 2

You can do this with more easily by manually creating a ValidationContext and then calling a Validator.TryValidateObject overload.

I think it's something like:

// using System.ComponentModel.DataAnnotations;

var validationContext = new ValidationContext(yourObject);
var validationResults = new List<ValidationResult>();
var isValid = Validator.TryValidate(yourObject, validationContext, validationResults, true);

Then you can loop through the validationResults and inspect which properties failed validation. This is the same mechanism that MVC uses (as far as I know) and means you can validate objects before they are attached to your data context.

Also strings for member names are prone to error, you may find this sort of method useful:

public static class ExpressionUtilities
{
    public static string GetPropertyName<T>(Expression<Func<T, object>> exp)
    {
        MemberExpression body = exp.Body as MemberExpression;
        if (body == null) 
        {
            throw new ArgumentException();
        }
        return body.Member.Name;
    }
}

Which means you can have type safe equivalents of this sort of thing:

error.PropertyName == "FirstName"

Instead would be:

error.PropertyName == ExpressionUtilities.GetPropertyName<Person>(p => p.Name)