0

So I have a boolean method that is used to verify if a command is valid. This is used inside of an engine in which it verifies that the process can continue or not. This is the validation method:

private bool CommandIsValid(WC command)
        {
            if (command.Address == null ||
                command.UserId < 0 ||
                String.IsNullOrEmpty(command.CurrencyCode) ||
                command.Amount < .01m ||
                command.Address.PrimitiveAddress == null ||
                String.IsNullOrEmpty(command.Address.Source) ||
                String.IsNullOrEmpty(command.Address.PrimitiveAddress.City) ||
                String.IsNullOrEmpty(command.Address.PrimitiveAddress.Country) ||
                String.IsNullOrEmpty(command.Address.PrimitiveAddress.FirstName) ||
                String.IsNullOrEmpty(command.Address.PrimitiveAddress.LastName) ||
                String.IsNullOrEmpty(command.Address.PrimitiveAddress.Region) ||
                command.Address.Created <= DateTime.MinValue)
            {
                return false;
            }
            return true;
        }

And is called here inside of my method here:

if (!CommandIsValid(cmd))
{
    _logger.Debug("Invalid command);
}

The issue is that I want to have some type of information regarding what failed validation. The best solution would have a list of what validations didn't pass, so I could relay that in my logging debugger. Obviously I could do this using a bunch of if-else statements, but it seems sloppy, as having a bunch of if else statements seems very poor style and I was wondering if there is any way in c# or in general I can do to avoid this.

10
  • Is that a typo? _logger.Debug("Invalid command); Sep 6, 2016 at 18:04
  • Instead of returning a bool, return a container of bool values when first is the overall status False/True then each one reflects a condition of the above. If first element is False, then you check which condition (index) is the false. Looks like it is fixed in size then you may just agree on the sequence. Sep 6, 2016 at 18:06
  • 1
    Another option is to move the validation logic inside the type, implementing some interface like IValidatableObject. Sep 6, 2016 at 18:07
  • No that is not a typo, _logger is just a logger. I'm not familiar with containers, do you mind adding a solution so i can attempt this? Thanks!
    – Varun Vu
    Sep 6, 2016 at 18:07
  • 2
    Your question is too broad. There are many different ways to approach this. You should start by trying something, and if it doesn't work for you for some reason, then explain why. It's not even clear here whether you would be satisfied simply knowing the first condition that failed, or if you want a way to report all of the conditions that failed. Sep 6, 2016 at 18:11

3 Answers 3

3

Are you familiar with DataAnnotations and it's associated Validator class?

It would require modifications to your object.

public PrimitiveAddress
{
[Required]
public string City {get;set;}
}

and then you use it like so:

var context = new ValidationContext(command.Address.PrimitiveAddress);
var results = new List<ValidationResult>();
var isValid = Validator.TryValidateObject(recipe, context, results);

if (!isValid)
{
    foreach (var validationResult in results)
    {
        Console.WriteLine(validationResult.ErrorMessage);
    }
}

if you've got a base command class you could probably add it in a more generic fashion. You can create your own validation attributes, use IValidatableObject for anything complex, customize the error messages

[Required(ErrorMessage="This is required.")]
0

Instead of returning a bool, return a container of bool values where first is the overall status False/True then each one reflects a condition of the above. If first element is False, then you check which condition (index) is the false. Looks like it is fixed in size then you may just agree on the sequence.

Something like this:

List<bool> YourFunction(YourDebuggerThing Input)
{
    List<bool> Result = new List<bool>();

    if (Input.Condition1 == false || Input.Condition2 == false || Input.Condition3 == false || Input.Condition4 == false || Input.Condition5 == false)
        Result.Add(false); // first element is always the overall status

    if(Input.Condition1 == false) Result.Add(false); else Result.Add(true); // element 2 is condition 1
    if(Input.Condition2 == false) Result.Add(false); else Result.Add(true); // element 3 is condition 2
    // ..
    // ConditionN

    return Result;
}
5
  • So this is sort of what I was thinking of, but it involves a bunch of if else statements. Is there any cleaner way to do this?
    – Varun Vu
    Sep 6, 2016 at 18:15
  • Of course there are. Plenty of ways. But this was from the top of my head. To find better ways we need to research Sep 6, 2016 at 18:16
  • Ok, because this is sort of the whole if-else thing I wanted to avoid in the first place
    – Varun Vu
    Sep 6, 2016 at 18:22
  • Validation rules are prone to change frequently due to new business requirements. With your code the developers not only have to maintain the validation function YourFunction but also its callers that rely on the ordering of bools inside the returned list. On a bigger scale this will turn into a maintenance nightmare. Sep 6, 2016 at 18:26
  • @GoodNightNerdPride As I mentioned in a reply comment to the OP's request of showing him how to do it. Definitely not the best nor the fanciest approach. Sep 6, 2016 at 18:28
0

One idea might be to implement your checks within the Get/Set methods of the class' properties, using a custom exception. Such as;

public class PrimitiveAddresses
{
    private string _city;
    public string City
    {
        get
        {
            if(_city != null) {return _city;}
            else {throw new CommandInvalidException("No valid city provided");}                 
        }
        set
        {
            _city = value;
        }

    }

}

public class CommandInvalidException: Exception
{
    public CommandInvalidException(string message)
    : base(message)
    {
    }
}

Then during you implementation, use a try/catch to handle the specific error;

public void foo()
{
    try
    {
        if (String.IsNullOrEmpty(command.Address.PrimitiveAddress.City)){} // Ect
    }
    catch (CommandInvalidException e)
    {
        Console.WriteLine("Command invalid due to " + e.message);
        // Or any other way you want to deal with the missing data
    }
}

Hope it helps :)

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.