HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Input validation for a user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inputuserforvalidation

Problem

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 validatin

Solution

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! 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();
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 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 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 results = _context.GetValidationErrors();

    var error = GetFirstError(results);

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

Code Snippets

foreach (DbEntityValidationResult result in results)
    foreach (DbValidationError error in result.ValidationErrors)
foreach (DbValidationError error in results.SelectMany(r => r.ValidationErrors))
var result =  results.FirstOrDefault(r => r.ValidationErrors.Any());
var error = result.ValidationErrors.First();
var errorMap = new Dictionary<string,Control>();
errorMap.Add("FirstName", FirstNameTextBox);
errorMap.Add("LastName", LastNameTextBox);
//etc..
// 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);
}

Context

StackExchange Code Review Q#114207, answer score: 13

Revisions (0)

No revisions yet.