Question: Best way / practice to organize code for validation method C#

Question

Best way / practice to organize code for validation method C#

Answers 1
Added at 2016-11-24 06:11
Tags
Question

I'm wondering what would be the best way or organize code when you doing the validation?

Nested if or return at the first place your validation failed?

This is the first way I did, focus on the successful scenario, using nested if

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        if (value!= null && value.GetType().Equals(typeof(string)))
        {
            var text = value.ToString();
            if (Regex.IsMatch(text, "^[-+]?[0-9]{1,2}.?[0-9]{0,6}?,[-+]?[0-9]{1,3}.?[0-9]{0,6}?$"))
            {
                var cordinations = text.Split(',');
                if (cordinations.Length == 2)
                {
                    decimal latitude = 0;
                    decimal longitude = 0;
                    if (decimal.TryParse(cordinations[0].Replace(" ", string.Empty), out latitude) && decimal.TryParse(cordinations[1].Replace(" ", string.Empty), out longitude))
                    {
                        if ((latitude >= -90 && latitude <= 90) && (longitude >= -180 && longitude <= 180))
                            return ValidationResult.Success;
                    }
                }
            }
        }
        return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));
    }

This is the second way I did, focus on the failed scenarios

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        if (value==null)
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));
        if (!value.GetType().Equals(typeof(string)))
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

        var text = value.ToString();
        if (!Regex.IsMatch(text, "^[-+]?[0-9]{1,2}.?[0-9]{0,6}?,[-+]?[0-9]{1,3}.?[0-9]{0,6}?$"))
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

        var cordinations = text.Split(',');
        if (cordinations.Length != 2)
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

        decimal latitude = 0;
        decimal longitude = 0;
        if (!decimal.TryParse(cordinations[0].Replace(" ", string.Empty), out latitude) || 
            !decimal.TryParse(cordinations[1].Replace(" ", string.Empty), out longitude))
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

        if (!(latitude >= -90 && latitude <= 90) || !(longitude >= -180 && longitude <= 180))
            return new ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName)); .
        return ValidationResult.Success;
    }

Which one would be considered as the best practice this day. Or any other way to do this. I would be really appreciate your ideas, may be any reference Microsoft suggests?

Answers
nr: #1 dodano: 2016-11-24 08:11

In my opinion, the second way is better as it prevent nesting and is a more friendly code, meaning the code can be easily maintained. However you could improve it may adding a ValidationResult at the top and update that instance at each validation to finally having only one return statement at the bottom.

What i mean is something like this:

protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
    var rtn = new ValidationResult();

    if (value==null)
        rtn  = ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));
    if (!value.GetType().Equals(typeof(string)))
        rtn =  ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

    var text = value.ToString();
    if (!Regex.IsMatch(text, "^[-+]?[0-9]{1,2}.?[0-9]{0,6}?,[-+]?[0-9]{1,3}.?[0-9]{0,6}?$"))
        rtn  = ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

    var cordinations = text.Split(',');
    if (cordinations.Length != 2)
        rtn  = ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

    decimal latitude = 0;
    decimal longitude = 0;
    if (!decimal.TryParse(cordinations[0].Replace(" ", string.Empty), out latitude) || 
        !decimal.TryParse(cordinations[1].Replace(" ", string.Empty), out longitude))
        rtn  = ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName));

    if (!(latitude >= -90 && latitude <= 90) || !(longitude >= -180 && longitude <= 180))
        rtn  =  ValidationResult(Admin.ResourceManager.GetString(ErrorMessageResourceName)); .

    if(rtn.HasNoError) rtn = ValidationResult.Success; // or something of the like

    return rtn;
}

This aids you in debugging by preventing the code from returning from somewhere in the middle of the method. Hope it helps

Source Show
◀ Wstecz