1
\$\begingroup\$

Here's a C# class as an example for object validation. For this purpose I decided to use .NET reflection and tried to use generics, probably not in the correct way, that's why I'm writing this post. I would really like to improve coding. Can you please give me your feedback / reviews / improvements or also new features to implement?

Please note that this post is already an improvement of this one.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Validation;
using System.Net.Mail;

namespace Validation
{
    class Program
    {
        public static bool CustomValidationLogic(object field)
        {
            return true;
        }
        private delegate bool validationDelegate(object field);

        static void Main(string[] args)
        {
            Customer customer = new Customer();
            Validation validationApp = new Validation(customer);

            Dictionary<string, FieldType> validationFields = validationApp.validationFields;
            validationFields["EMail"] = FieldType.EMail;
            validationFields["DateOfBirth"] = FieldType.DateTime;
            validationFields["Age"] = FieldType.Numeric;
            validationFields["PurchasedItems"] = FieldType.Numeric;
            validationFields["DateLastPurchase"] = FieldType.DateTime;
            validationFields["Age"] = FieldType.Integer;

            // the following two lines are optional (override the default validation rule)
            validationApp.customValidationHandlers = new Hashtable();
            validationApp.customValidationHandlers.Add(FieldType.EMail, (validationDelegate)CustomValidationLogic);

            object validationResult = validationApp.setValidationFields(validationFields).doValidation();
        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Reflection;

namespace Validation
{
    class Customer
    {
        public object EMail { get; set; }
        public object DateOfBirth { get; set; }
        public object Age { get; set; }
        public object PurchasedItems { get; set; }
        public object DateLastPurchase { get; set; }

        public Customer() {
            this.EMail = "[email protected]";
            //this.DateOfBirth = DateTime.Parse("06/06/1981");
            this.DateOfBirth = "01/12/1981";
            this.Age = "36";
            this.PurchasedItems = "3";
            this.DateLastPurchase= "06/06/2017";
        }
    }
}

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Reflection;
using System.Text.RegularExpressions;

namespace Validation
{
    public enum FieldType
    {
        EMail,
        DateTime,
        Numeric,
        Integer
    }

    public class ValidationResult
    {
        public bool isValid { get; set; }
        public string invalidPropertyName { get; set; }
        public string invalidFieldName { get; set; }
        public string invalidPropertyValue { get; set; }

        public ValidationResult()
        {
            this.isValid = false;
            this.invalidPropertyName = string.Empty;
            this.invalidFieldName = string.Empty;
            this.invalidPropertyValue = string.Empty;
        }
    }

    public class Validation
    {
        private object _toBeValidated { get; set; }
        public Dictionary<string, FieldType> validationFields { get; set; }

        public Hashtable customValidationHandlers { get; set; }

        public Validation(object o)
        {
            this._toBeValidated = o;
            Dictionary<string, FieldType> d = new Dictionary<string, FieldType>();

            this.validationFields = new Dictionary<string, FieldType>();
        }

        public Validation setValidationFields(Dictionary<string, FieldType> f)
        {
            this.validationFields = f;
            return this;
        }

        public ValidationResult doValidation()
        {
            ValidationResult result = new ValidationResult();

            Dictionary<string, FieldType> fields = this.validationFields;
            foreach (PropertyInfo property in this._toBeValidated.GetType().GetProperties())
            {
                FieldType field_type;
                field_type = fields[property.Name];
                if (fields.ContainsKey(property.Name))
                {
                    var propertyValue = property.GetValue(this._toBeValidated, null);
                    result.isValid = ValidateField(propertyValue, field_type);
                    if (!result.isValid)
                    {
                        result.invalidPropertyName = property.Name;
                        result.invalidFieldName = field_type.ToString();
                        result.invalidPropertyValue = propertyValue.ToString();
                        break;
                    }
                }
            }
            return result;
        }

        private Boolean ValidateField(object field, FieldType type)
        {
            Boolean isValid = false;
            switch (type)
            {
                case FieldType.EMail:
                    isValid = (ValidationDefaultRules.ValidateEMail(field) && ValidationCustomRules.ValidateEMail(this, field));
                    break;

                case FieldType.DateTime:
                    DateTime d;
                    isValid = DateTime.TryParse(field.ToString(), out d);
                    if (d > DateTime.Now) isValid = false;
                    break;

                case FieldType.Numeric:
                    isValid = new Regex(@"^[-+]?(\d*\.)?\d+$").IsMatch(field.ToString());
                    break;

                case FieldType.Integer:
                    int nAsInteger;
                    isValid = int.TryParse(field.ToString(), out nAsInteger);
                    break;

            }
            return isValid;
        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net.Mail;

namespace Validation
{
    public class ValidationCustomRules
    {
        public static bool ValidateEMail(Validation validation, object field)
        {
            FieldType field_type = FieldType.EMail;

            // override the default validation for EMail
            if (validation.customValidationHandlers != null)
            {
                Delegate handler = (Delegate)validation.customValidationHandlers[field_type];
                if (handler != null)
                    return (bool)handler.DynamicInvoke(field);
                else
                    return true;
            }
            else
                return true;
        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net.Mail;

namespace Validation
{
    public class ValidationDefaultRules
    {
        public static bool ValidateEMail(object value) {
            try
            {
                MailAddress m = new MailAddress((string)value);
                return true;
            }
            catch (FormatException)
            {
                return false;
            }
        }
    }
}
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

I wouldn't say this is an improvement. The Customer class has worsened since last time.

public object EMail { get; set; }
public object DateOfBirth { get; set; }
public object Age { get; set; }
public object PurchasedItems { get; set; }
public object DateLastPurchase { get; set; }

Now everything is an object where before it were concrete types:

public string EMail { get; set; }
public DateTime DateOfBirth { get; set; }
public object Age { get; set; }
public object PurchasedItems { get; set; }
public object DateLastPurchase { get; set; }

Why don't you use real types for the data but strings that you store as object?


Another thing is the naming. Your properties are extremely inconsistent. Once you use PascalCase for such names as DateOfBirth only to use camelCase for invalidPropertyName. Or you use PascalCase for private members (ValidateField) and camelCase for public ones (doValidation) where the other way around wound be better (if you insist on using camelCase) although we actually start all method names with an uppercase letter.


I think you should read @Pieter Witvoet's answer a few more times.


Other then this your ValidateField method would look better with an early return rather then with the helper variable isValid.

\$\endgroup\$
3
  • \$\begingroup\$ Ok, I'm gonna do more changes to the naming convention, and other things. But this code is an improvement, if you look carefully at the code. The object properties cannot be the real data type, because I want my Validation class to take a generic object as input , and validate it based on default / custom rules \$\endgroup\$
    – Gae
    Oct 15, 2017 at 20:57
  • \$\begingroup\$ @Gae believe me, this is just wrong. You wouldn't have to validate any of the properties if they were strongly typed. What is it that you validate? If a string is an email? Ok. If a date-time is greater than now? Ok, but why do you have to parse it first? Then you validate if a number is a number or if an int is an int. This wouldn't be necessary if they were actually numbers. \$\endgroup\$
    – t3chb0t
    Oct 16, 2017 at 3:47
  • \$\begingroup\$ ok, but to better understand what you mean I d need to have your code version, so that I can better study your point of view. \$\endgroup\$
    – Gae
    Oct 16, 2017 at 6:28
1
\$\begingroup\$

Could you explain why to not write your customer class in this way?.

class Customer
{
    public string Mail { get; set; }
    public DateTime DateOfBirth { get; set; }
    public uint Age { get; set; } //This is redudant or should be calculated
    public uint PurchasedItems { get; set; } //I assume is an integer bigger than 0
    public DateTime DateLastPurchase { get; set; }
}

If you write it so, then you enforce most of your "validation" as the properties are strongly typed. In 99.9% of the cases you want a validation system for bussines rules, for example, age is between 0 a 150 and not to enforce a type.

\$\endgroup\$
1
  • \$\begingroup\$ Well, I want to give the freedom to pass not strongly typed parameters, and force the validation to be based only on FieldType.XXX , in this way I can pass a string, as well a DateTime or whatever. But, if you have a better idea / implementation can you give me your code version please ? I understand better what you mean in that way, thanks \$\endgroup\$
    – Gae
    Oct 18, 2017 at 14:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.