1
\$\begingroup\$

This class accepts input parameters (Login and Password) from ApiController via POST method, and do the following:

  1. Trims the white spaces.
  2. Checks for not null and Not Empty.
  3. Checks the length of the input string to ensure it hasn't exceeded (only if the item 2 exceeds).

But, I think the class is violating the DRY Principle.

using FluentValidation;
using FluentValidation.Attributes;

namespace BusinessService.Auth.Models
{
    [Validator(typeof(AuthModelValidator))]
    public sealed class AuthPostModel
    {
        string _username;
        public string Username
        {
            get { return _username; }
            set { _username = value.Trim(); }
        }

        string _password;
        public string Password
        {
            get { return _password; }
            set { _password = value.Trim(); }
        }
    }
    class AuthModelValidator : AbstractValidator<AuthPostModel>
    {
        public AuthModelValidator()
        {
            RuleFor(x => x.Username).NotEmpty();
            RuleFor(x => x.Username).Length(1, 256)
                .When(x => !string.IsNullOrEmpty(x.Username));

            RuleFor(x => x.Password).NotEmpty();
            RuleFor(x => x.Password).Length(1, 32)
                .When(x => !string.IsNullOrEmpty(x.Password));
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Where do you think your code violates the DRY principle exactly? \$\endgroup\$ – t3chb0t Nov 12 '17 at 8:05
  • \$\begingroup\$ Trim(), .NotEmpty(), Length(n, n) and When(x => !string.IsNullOrEmpty(n)) can be wrapped into something common? \$\endgroup\$ – Coder Absolute Nov 12 '17 at 8:27
  • 2
    \$\begingroup\$ I don't see any DRY violations there. You do four different things on two different properties. I'd leave it as is. If you wrap it and want to define a different rule later for either property then you'll need to unwrap it agian. I find the code is fine. Although When seems to be redundant after NotEmpty that already makes sure that the exact same condition is true. \$\endgroup\$ – t3chb0t Nov 12 '17 at 8:33
  • \$\begingroup\$ @t3chb0t Thanks for letting me know. It's always a good idea to get it verified by someone who knows more than I do. \$\endgroup\$ – Coder Absolute Nov 12 '17 at 10:56
  • \$\begingroup\$ No, it's not redundant. If I don't add the When clause and check for non empty string then am going to get two errors if the password field is blank, for an example: 'Password' should not be empty. and 'Password' must be between 1 and 32 characters. You entered 0 characters. \$\endgroup\$ – Coder Absolute Nov 12 '17 at 11:02

Your Answer

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

Browse other questions tagged or ask your own question.