6
\$\begingroup\$

I am using fluentvalidation to validate my object model. I have to make sure that the first two characters of an ISIN must be letters / non numeric. The way I did it is to convert the string to a char array and then perform regex on the first two characters. I was wondering if anyone has a nicer way of doing the validation using a custom fluent validation. Thanks

    public class Create
    {
        public class Command : IRequest
        {
            public string Name { get; set; }

            public string Exchange { get; set; }

            public string Ticker { get; set; }

            public string Isin { get; set; }

            public string Website { get; set; }
        }

        public class CommandValidator : AbstractValidator<Command>
        {
            public CommandValidator()
            {
                RuleFor(x => x.Name).NotEmpty();
                RuleFor(x => x.Isin).SetValidator(new MyValidator());
            }
        }

        public class MyValidator : PropertyValidator
        {
            public MyValidator(
                string errorMessage = "Isin. The first two characters of an ISIN must be letters / non numeric.") : base(errorMessage)
            {
            }

            protected override bool IsValid(PropertyValidatorContext context)
            {
                var stringToValidate = context.PropertyValue as String;
                return IsValid(stringToValidate);
            }

            public bool IsValid(string stringToValidate)
            {
                var charString = stringToValidate.ToCharArray();
                if (Regex.IsMatch(charString[0].ToString(), "[0-9]") || Regex.IsMatch(charString[1].ToString(), "[0-9]"))
                {
                    return false;
                }

                return true;
            }

            public class Handler : IRequestHandler<Command>
            {
                private readonly DataContext _context;

                public Handler(DataContext context)
                {
                    _context = context;
                }
                public async Task<Unit> Handle(Command request,
                    CancellationToken cancellationToken)
                {
                    var company = new Company
                    {
                        Name = request.Name,
                        Exchange = request.Exchange,
                        Ticker = request.Ticker,
                        Isin = request.Isin,
                        Website = request.Website
                    };

                    await _context.Companies.AddAsync(company);

                    var success = await _context.SaveChangesAsync() > 0;

                    if (success) return Unit.Value;

                    throw new Exception("Problem saving changes");
                }
            }
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$
public bool IsValid(string stringToValidate)
{
    var charString = stringToValidate.ToCharArray();
    if (Regex.IsMatch(charString[0].ToString(), "[0-9]") || Regex.IsMatch(charString[1].ToString(), "[0-9]"))
    {
        return false;
    }
    return true;
}

Be aware that each char in a string can be reached by index as stringToValidate[0], so you don't need to call ToCharArray():

public bool IsValid(string stringToValidate)
{        
    if (Regex.IsMatch(stringToValidate[0].ToString(), "[0-9]") || Regex.IsMatch(stringToValidate[1].ToString(), "[0-9]"))
    {
        return false;
    }
    return true;
}

But Char has a static method Char.IsLetter() that can be used instead of the Regex expression:

public bool IsValid(string stringToValidate)
{
     return stringToValidate.Length > 1 && Char.IsLetter(stringToValidate[0]) && Char.IsLetter(stringToValidate[1]);
}

This is more consistent with modern unicode character sets, and it will also catch if the two first characters are special chars like '&' or '!' etc. If you just want to ensure they are not numeric, you can use Char.IsDigit() instead.

\$\endgroup\$
1
  • \$\begingroup\$ @user228313 Just a small addition. As always measure different solutions before you change your current to another one. In this test the ASCII character check outperformed every other solution. \$\endgroup\$ Jul 28 '20 at 14:39
3
\$\begingroup\$

Why are you creating a new variable, and then immediately using it in the base() constructor?

public MyValidator(
    string errorMessage = "Isin. The first two characters of an ISIN must be letters / non numeric.") : base(errorMessage)
{
}

For readability think about moving this directly to base() like so:

public MyValidator() : base("Isin. The first two characters of an ISIN must be letters / non numeric.")
{
}

Instead of declaring stringToValidate as a string, then using it's value, you could improve conciseness by moving the expression immediately into the function call, like so:

protected override bool IsValid(PropertyValidatorContext context)
{
    return IsValid(context.PropertyValue as String);
}

Instead of the if returning false or true manually, just return the inverse result of the statement, like this:

public bool IsValid(string stringToValidate)
{
    var charString = stringToValidate.ToCharArray();
    return !Regex.IsMatch(charString[0].ToString(), "[0-9]") || Regex.IsMatch(charString[1].ToString(), "[0-9]");
}

For conciseness, you could rewrite a few lines in the last method (Handler.Handle()) like so:

public class Handler : IRequestHandler<Command>
{
    private readonly DataContext _context;

    public Handler(DataContext context)
    {
        _context = context;
    }
    public async Task<Unit> Handle(Command request,
        CancellationToken cancellationToken)
    {
        await _context.Companies.AddAsync(new Company
        {
            Name = request.Name,
            Exchange = request.Exchange,
            Ticker = request.Ticker,
            Isin = request.Isin,
            Website = request.Website
        );

        if (await _context.SaveChangesAsync() <= 0)
        {
            throw new Exception("Problem saving changes");
        }

        return Unit.Value
    }
}

Also, note that I switched the order of the final few lines. You should be checking for false results/exceptions in the indented code, but returning the actual result at the bottom. For more information on reducing nesting, read this.

\$\endgroup\$

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.