HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Refactor a selection validator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
validatorrefactorselection

Problem

I have a data table and have to validate every field in it. I have refactor this code to this below, but the complexity is 15(!!) Should I make something like dictionary with type as Key and Func as Value?

private bool CheckField(DataRow dataRow, ValidationField validationField)
        {
            bool result = false;
            if (validationField.Requiered)
            {
                if (validationField.Type == typeof (int))
                {
                    result = this.CheckIntegerAndNotNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof (DateTime))
                {
                    result = this.CheckDateTimeAndNotNull(dataRow[validationField.Name].ToString());
                }
            }
            else
            {
                if (validationField.Type == typeof (int))
                {
                    result = this.CheckIntegerOrNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof(DateTime))
                {
                    result = this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof(string))
                {
                    result = this.CheckStringOrNull(dataRow[validationField.Name].ToString(),
                        validationField.MaxLength.Value);
                }
                else if (validationField.Type == typeof(decimal))
                {
                    result = this.CheckDecimalOrNull(dataRow[validationField.Name].ToString());
                }
            }
            return result;
        }

 public class ValidationField
    {
        public Type Type { get; set; }
        public string Name { get; set; }
        public bool Requiered  { get; set; }
        public int? MaxLength { get; set; }
    }

Solution

The first simplification is for the Required check. All you need to do here is check that the value is not null, you don't care about the type. Then you can do the type-specific check later. So remove the outermost if...else and simply have:

if (validationField.Requiered && DBNull.Value.Equals(validationField.Name]))
    return false;


The next section is a bit more difficult. However, one approach would be something like:

var dataString = dataRow[validationField.Name].ToString();
return dataString == Convert.ChangeType(dataString, validationField.Type).ToString();


What we're doing here is using .NET's Convert.ChangeType method to change from the string to your desired primative type. Then, if the string representation of the converted item exactly matches the original string, we say that the string must exactly represent the converted item, meaning it is of the correct data type. You may want to also pass in an IFormatProvider for additional safety.

Putting those two code blocks together would pretty much give you your full method. You may need one additional check for the max length of a string though.

Other than the above alternatives, your code mostly looks good. One thing I'd do is scrap the result variable, and return directly from inside your if statements. The this. prefixes are also unnecessary for PascalCased method names.

As a more general design note, it might be preferable to do this validation at a different point in your process. For example, DataColumns can be set to require a particular Type, and can also be told whether or not to accept null values using the AllowDBNull property, meaning the table will largely do this validation for you.

Code Snippets

if (validationField.Requiered && DBNull.Value.Equals(validationField.Name]))
    return false;
var dataString = dataRow[validationField.Name].ToString();
return dataString == Convert.ChangeType(dataString, validationField.Type).ToString();

Context

StackExchange Code Review Q#57660, answer score: 8

Revisions (0)

No revisions yet.