3
\$\begingroup\$

This code snippet is intended for validating the value of a property of an object. The min and max range is supplied as an .xml file like Nhibernate .hbm files. Since the data-type of the property is also read from the .xml file, we can only know the type of the property at run-time.

Is there any better way of improving the code in C# and .NET 2.0?

public static void ValidateMinMax(Property prop, Object value)
    {
        Type type = Type.GetType(prop.TypeName);

        Object minValue = PropertyDataExtractor.GetMinValue(prop);
        Object maxValue = PropertyDataExtractor.GetMaxValue(prop);
        Object actualVaue = null;

        bool minValueOk = false;
        bool maxValueOk = false;

        if (minValue != null)
        {
            switch (type.Name)
            {
                case "Boolean":
                    break;

                case "SByte":
                    actualVaue = Convert.ToSByte(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((sbyte)minValue) <= ((sbyte)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Byte":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((byte)minValue) <= ((byte)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Byte[]":
                    break;

                case "DateTime":
                    actualVaue = Convert.ToDateTime(value);

                    if (actualVaue != null)
                    {
                        minValueOk = ((DateTime)minValue).Date <= ((DateTime)actualVaue).Date;
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Int16":
                    actualVaue = Convert.ToInt16(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((short)minValue) <= ((Int16)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Int32":
                    actualVaue = Convert.ToInt32(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((int)minValue) <= ((Int32)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Int64":
                    actualVaue = Convert.ToInt64(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((long)minValue) <= ((Int64)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Single":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((float)minValue) <= ((Single)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Double":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((double)minValue) <= ((double)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Decimal":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((decimal)minValue) <= ((decimal)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Char":
                    actualVaue = Convert.ToChar(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((char)minValue) <= ((char)actualVaue);
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "String":
                    actualVaue = Convert.ToString(value);
                    if (actualVaue != null)
                    {
                        minValueOk = ((int)minValue) <= ((string)actualVaue).Length;
                    }
                    else
                    {
                        minValue = true;
                    }
                    break;

                case "Guid":
                    break;
            }
        }

        if (maxValue != null)
        {
            switch (type.Name)
            {
                case "Boolean":
                    break;

                case "SByte":
                    actualVaue = Convert.ToSByte(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((sbyte)minValue) >= ((sbyte)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Byte":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((byte)minValue) >= ((byte)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Byte[]":
                    break;

                case "DateTime":
                    actualVaue = Convert.ToDateTime(value);
                    if (actualVaue != null)
                    {                            
                        maxValueOk = ((DateTime)maxValue).Date >= ((DateTime)actualVaue).Date;
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Int16":
                    actualVaue = Convert.ToInt16(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((short)minValue) >= ((Int16)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Int32":
                    actualVaue = Convert.ToInt32(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((int)minValue) >= ((Int32)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Int64":
                    actualVaue = Convert.ToInt64(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((long)minValue) >= ((Int64)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Single":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((float)minValue) >= ((Single)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Double":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((double)minValue) >= ((double)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Decimal":
                    actualVaue = Convert.ToByte(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((decimal)minValue) >= ((decimal)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Char":
                    actualVaue = Convert.ToChar(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((char)minValue) >= ((char)actualVaue);
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "String":
                    actualVaue = Convert.ToString(value);
                    if (actualVaue != null)
                    {
                        maxValueOk = ((int)maxValue) >= ((string)actualVaue).Length;
                    }
                    else
                    {
                        maxValue = true;
                    }
                    break;

                case "Guid":
                    break;
            }
        }

        if (minValue == null)
        {
            minValueOk = true;
        }

        if (maxValue == null)
        {
            maxValueOk = true;
        }

        if (!(minValueOk && maxValueOk))
        {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) +".\nMessage : "+ PropertyDataExtractor.GetMinValueErrorMessage(prop) + " " + PropertyDataExtractor.GetMaxValueErrorMessage(prop));
        }
        else if (!minValueOk)
        {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) + ".\nMessage : " + PropertyDataExtractor.GetMinValueErrorMessage(prop));
        }
        else if (!maxValueOk)
        {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) + ".\nMessage : " + PropertyDataExtractor.GetMaxValueErrorMessage(prop));
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ You seem to duplicate your switch statements twice. Perhaps using polymorphism might be an approach worth considering here? \$\endgroup\$ – dreza Nov 3 '12 at 7:30
  • \$\begingroup\$ What is the value of the parameter "value"? You are using Convert, which is used to convert from one type to another. But it looks like it should either be the type you need to deal with already, or a string/xml fragment. \$\endgroup\$ – jmoreno Nov 3 '12 at 17:11
  • 1
    \$\begingroup\$ There's a lot of logical errors in that code... normally we would prefer actual working code. \$\endgroup\$ – Jeff Mercado Nov 3 '12 at 17:12
3
\$\begingroup\$

Here's an attempt to use a bit of polymorphism. While doing this some things I did note were:

1) The if statement at the end means that !minValueOk and !maxValueOk are irrelevant and will never be true. The first if should be !minValueOk and !maxValueOk rather than !(minValueOk && maxValueOk.

2) Assuming value is never null for some of the checks you do not need to double check for null. For example Convert.ToSByte(value) should always return a non-null value assuming it will convert.

3) I typically try not to through Exception where I can. Perhaps a custom exception might be worthwhile here.

Here's my attempt at a refactor. I'm not 100% sure it will compile but it should at least give an idea:

The new Validate method:

public static void ValidateMinMax(object prop, Object value)
{
    PropertyValidator validator = new PropertyValidatorFactory().CreateValidator(prop.GetType().Name);

    if (!validator.Validate(prop, value))
    {
        if (!isMinValid && !isMaxValid)
        {
             message = PropertyDataExtractor.GetMinValueErrorMessage(prop) + " " + PropertyDataExtractor.GetMaxValueErrorMessage(prop);
        }
        else if (!isMinValid)
        {
             message = PropertyDataExtractor.GetMinValueErrorMessage(prop);
        }
        else
        {
             message = PropertyDataExtractor.GetMaxValueErrorMessage(prop);
        }

        return InvalidPropertyRangeException("Property : " + PropertyDataExtractor.GetName(prop) + ".\nMessage : " + message);
    }
}

Using a custom exception and a Factory class to create the objects that will do the validation

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

class PropertyValidatorFactory
{
    public PropertyValidator CreateValidator(string typeName)
    {
        switch (typeName)
        {
            case "Boolean":
                return new EmptyValidator();
            case "String":
                return new StringValidator();
            // Rest of validators go here
            default:
                throw new NotImplementedException(string.Format("Type {0} is not supported", typeName));
        }
    }
}

The base validator class

    abstract class PropertyValidator
    {
    public bool IsMinValid { get; private set; }
    public bool IsMaxValid { get; private set; }

    protected Object OriginalValue { get; private set; }

    protected PropertyValidator()
    {
        IsMinValid = IsMaxValid = true;
    }

    public bool Validate(object prop, Object value)
    {
        if(value == null)
        {
            throw new NullReferenceException("Value supplied for validation is null");
        }

        Object minValue = PropertyDataExtractor.GetMinValue(prop);
        Object maxValue = PropertyDataExtractor.GetMaxValue(prop);

        if(minValue == null && maxValue == null)
        {
            return true;
        }

        OriginalValue = value;

        return IsValid(minValue, maxValue);
    }

    private bool IsValid(object minValue, object maxValue)
    {
        IsMinValid = ValidateMinimum(minValue);
        IsMaxValid = ValidateMaximum(maxValue);

        return IsMinValid && IsMaxValid;
    }

    protected abstract bool ValidateMinimum(object minValue);
    protected abstract bool ValidateMaximum(object maxValue);
}

Examples of specific validator classes

class EmptyValidator : PropertyValidator
{
    protected override bool ValidateMinimum(object minValue)
    {
        return true;
    }

    protected override bool ValidateMaximum(object maxValue)
    {
        return true;
    }
}

class SbyteValidator : PropertyValidator
{
    protected override bool ValidateMinimum(object minValue)
    {
        sbyte actualVaue = GetValue();

        return ((sbyte)minValue) <= actualVaue;            
    }

    protected override bool ValidateMaximum(object maxValue)
    {
        sbyte actualVaue = GetValue();

        return ((sbyte)maxValue) >= actualVaue;
    }

    private sbyte GetValue()
    {
        return Convert.ToSByte(OriginalValue);
    }
}

class StringValidator : PropertyValidator
{
    protected override bool ValidateMinimum(object minValue)
    {
        string actualVaue = GetValue();
        if (actualVaue != null)
        {
            return ((int)minValue) <= actualVaue.Length;
        }

        return false;

    }

    protected override bool ValidateMaximum(object maxValue)
    {
        string actualVaue = GetValue();
        if (actualVaue != null)
        {
            return ((int)maxValue) >= actualVaue.Length;
        }

        return false;
    }

    private string GetValue()
    {
        return Convert.ToString(OriginalValue);
    }
}
\$\endgroup\$
1
\$\begingroup\$

This might be a good place to use the strategy pattern. By using the strategy pattern each method is independent of each other. It does end up being more code but it also separates the code but it honors the Single Responsibility Prinicipal, and is much easier more flexible (easy to add additional types in the future), easier to understand and maintain, and easier to unit test. You can easily comparing additional types just by addings another class and registering it, instead of continuing to have to add items to a switch statement and creating one huge method.

(Note this was written in NotePad so it probably is not perfect, but should give you a good idea).

public interface IValidateMinMax
{
    bool ValidateMin(object minValue, object value);
    bool ValidateMax(object maxValue, object value);
}

public static class Validator
{
    public static bool ValidateMin<T>(T minValue, T value) where T : struct
    {
        return ((T)minValue) <= ((T)actualVaue);
    }
    public static bool ValidateMax<T>(T minValue, T value) where T : struct
    {
        return ((T)minValue) >= ((T)actualVaue);
    }
}

public class ValidateMinMaxSByte : IValidateMinMax
{
    public bool ValidateMin(object minValue, object value)
    {
        return Validator.ValidateMin<sbyte>(minValue, value);
    }

    public bool ValidateMax(object maxValue, object value)
    {
        return Validator.ValidateMax<sbyte>(minValue, value);
    }
}

public class ValidateMinMaxByte : IValidateMinMax
{
    public bool ValidateMin(object minValue, object value)
    {
        return Validator.ValidateMin<byte>(minValue, value);
    }

    public bool ValidateMax(object maxValue, object value)
    {
        return Validator.ValidateMax<byte>(minValue, value);
    }
}

public class ValidateMinMaxDateTime : IValidateMinMax
{
    public bool ValidateMin(object minValue, object value)
    {
        return Validator.ValidateMin<DateTime>(minValue, value);
    }

    public bool ValidateMax(object maxValue, object value)
    {
        return Validator.ValidateMax<DateTime>(minValue, value);
    }
}

etc...

Then you would create the class which would register all the validators and call the appropriate one...

public class ValidatorMinMaxController
{
public IDictionary<Type, IValidateMinMax> _validators = new Dictionary<Type, IValidateMinMax>();
public void Register()
{
    _validators.Add(typeof(sbyte), new ValidateMinMaxSByte());
    _validators.Add(typeof(byte), new ValidateMinMaxByte());
    _validators.Add(typeof(DateTime), new ValidateMinMaxDateTime());
              etc...
}

public void ValidateMinMax(Property prop, Object value)
{
    Type type = Type.GetType(prop.TypeName);

    Object minValue = PropertyDataExtractor.GetMinValue(prop);
    Object maxValue = PropertyDataExtractor.GetMaxValue(prop);
    Object actualVaue = null;

    bool minValueOk = true;
    bool maxValueOk = true;

    IValidateMinMax instance;
    if (_validators.TryGetValue(type, out instance))
    {
            if (minValue != null)
                minValueOk = instance.ValidateMin(minValue, value);

            if (maxValue != null)
                maxValueOk = instance.ValidateMax(maxValue, value);
    }

    if (!(minValueOk && maxValueOk))
    {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) +".\nMessage : "+ PropertyDataExtractor.GetMinValueErrorMessage(prop) + " " + PropertyDataExtractor.GetMaxValueErrorMessage(prop));
    }
    else if (!minValueOk)
    {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) + ".\nMessage : " + PropertyDataExtractor.GetMinValueErrorMessage(prop));
    }
    else if (!maxValueOk)
    {
            throw new Exception("Property : " + PropertyDataExtractor.GetName(prop) + ".\nMessage : " + PropertyDataExtractor.GetMaxValueErrorMessage(prop));
    }                   
}
\$\endgroup\$
  • \$\begingroup\$ Will the TryGetValue work on type even though it's a dictionary of strings? Would you not ant type.Name? I do like this pattern though. I've seen it used quite a bit in Microsoft MVC's framework. \$\endgroup\$ – dreza Nov 5 '12 at 19:34
  • \$\begingroup\$ OH...looks like I got my types intermingled. You can either use the type or string. But I should code one or the other, not both :) Code has been fixed. \$\endgroup\$ – Gene S Nov 5 '12 at 19:57
1
\$\begingroup\$

This is the snippet, add appropriate run-time checks and you're good to go:

public enum ErrorCode { Undef = -1, OK, TooSmall, TooBig, Err }

public class MyValidator
{
    private static Type ValidateAs(Type type)
    {
        if (type.Equals(typeof(String)))
            return typeof (Int32);
        return type;
    }

    private static object TypeCheckConvertion(object obj)
    {
        if (obj is DateTime)
            return ((DateTime)obj).Date;            
        if (obj is String)
            return (Int32) (obj.ToString().Length);         
        return obj;
    }

    public static ErrorCode ValidateProperty<T>(T min, T max, T val) where T : IComparable
    {
        Console.WriteLine(String.Format("Is {0} in range [{1}-{2}] ?", val, min, max));
        ErrorCode res = ErrorCode.OK;           
        if (val.CompareTo(min) < 0)
            res |= ErrorCode.TooSmall;
        if (val.CompareTo(max) > 0)
            res |= ErrorCode.TooBig;            
        return res;
    }

    public static ErrorCode ValidateMinMax(Property prop, Object obj)
    {
        var type = ValidateAs(Type.GetType(prop.TypeName));
        var minValue = TypeCheckConvertion(Property.GetMinValue(prop));
        var maxValue = TypeCheckConvertion(Property.GetMaxValue(prop));
        var val = TypeCheckConvertion(obj);

        return (ErrorCode) typeof(MyValidator).GetMethod("ValidateProperty").MakeGenericMethod(
            new [] {type}).Invoke(null, new[] { minValue, maxValue, val });
    }
}
\$\endgroup\$
  • \$\begingroup\$ Would this work also for the String validation, given that it is working off .Length? \$\endgroup\$ – dreza Nov 6 '12 at 2:07
  • \$\begingroup\$ Perhaps you could expand on your post how your answer covers that scenario just to cover all bases? \$\endgroup\$ – dreza Nov 6 '12 at 3:02
  • \$\begingroup\$ @dreza: I've edited the code to cover all mentioned cases. \$\endgroup\$ – avip Nov 6 '12 at 5:31
  • \$\begingroup\$ no prob. I was just interested to see where it was going given string had slightly different requirements. Interesting approach. \$\endgroup\$ – dreza Nov 6 '12 at 6:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy