3
\$\begingroup\$

I have a simple class to ensure the date entered into a form is a valid date. Where can I improve it, have I missed anything?

public class ValidDate : ValidationAttribute
{
    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        var dateToParse = value.ToString();
        var parsedDate = new DateTime();

        if (DateTime.TryParseExact(dateToParse, "dd/MM/yyyy", System.Globalization.CultureInfo.InvariantCulture,
            System.Globalization.DateTimeStyles.None, out parsedDate))
            return ValidationResult.Success;

        return new ValidationResult("Invalid date, please try again with a valid date in the format of DD/MM/YYYY.");
    }
}

Thanks,

\$\endgroup\$
2
  • 2
    \$\begingroup\$ I would consider to include the malformed input data into the error message. It can help a lot during investigation. \$\endgroup\$ Jun 24, 2020 at 13:41
  • 1
    \$\begingroup\$ @PeterCsala that's a good shout, thanks, I'll add that in. \$\endgroup\$
    – MF1010
    Jun 25, 2020 at 7:33

2 Answers 2

4
\$\begingroup\$

You could easily turn your method into a one-liner expression-bodied method with a few tricks:

  • use the _ discard variable instead of the explicit parsedDate variable (also, it doesn't need initializing to new DateTime()).
  • use the conditional operator since you're returning one of two ValidationResults.
  • value.ToString() can be passed directly into TryParseExact without the extra local variable dateToParse.

Giving:

protected override ValidationResult IsValid(object value, ValidationContext validationContext) =>
    DateTime.TryParseExact(value.ToString(), "dd/MM/yyyy", System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out _)
        ? ValidationResult.Success
        : new ValidationResult("Invalid date, please try again with a valid date in the format of DD/MM/YYYY.");

Couple more items:

  • Recommend -- for ease of reading -- adding a using System.Globalization; to your namespace so you don't have to type that long prefix out twice.
  • As this is an attribute, add the suffix Attribute to your class name.
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Great stuff, thanks. \$\endgroup\$
    – MF1010
    Jun 24, 2020 at 12:46
1
\$\begingroup\$

I think other answer had some great points, also I would use fluent validation instead so your model does not need to have dependencies on validation attributes.

https://docs.fluentvalidation.net/

\$\endgroup\$
1
  • \$\begingroup\$ I'll check that out, thanks. \$\endgroup\$
    – MF1010
    Jun 25, 2020 at 7:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.