Validation for Model Object using Fluent Validation

Question

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");
 }
 }
 }
 }
}

Answer 1

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.

• Peter Csala: @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.

Answer 2

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.