Code Improvement: Copy Regex named capture groups to strongly typed object

Question

This code works to copy named capture groups of a Regex to the same named properties of a strongly typed object. However, it's not pretty. Someone mentioned I should use polymorphism to at least remove the five if statements.

What should I consider when improving this code? Suggested reading?

// Copies named capture groups in a REGEX to the properties of an object
// if the names match
static public void CopyProperties(Regex regex, Match source, object target)
{
    if (target != null && source != null && source.Success)
    {
        var targetType = target.GetType();
        // only step through non numeric capture names
        foreach (var sourceProp in regex.GetGroupNames()
            .Where(p => !Regex.IsMatch(p, @"^\d+$") && 
            !String.IsNullOrWhiteSpace(p)))
        {
            // does the target property the same as source exist?
            var targetPropName = targetType.GetProperty(sourceProp);
            // if the source and target exist
            if (targetPropName != null &&
                !String.IsNullOrWhiteSpace(source.Groups[sourceProp].Value) )
            {
                var targetPropSetter = targetPropName.GetSetMethod();
                // created a variable for each possible type
                // because I couldn't inline the Convert.ToInt32, etc.
                string sourceValue = source.Groups[sourceProp].Value;
                int? intValue = !Regex.IsMatch(sourceValue, 
                                  @"^\d{1,10}$")
                    ? (int?) null 
                    : Convert.ToInt64(sourceValue) <= Int32.MaxValue
                        ? Convert.ToInt32(sourceValue) 
                        : (int?) null;
                long? longValue = Regex.IsMatch(sourceValue, 
                                  @"^\d{1,19}$")
                    ? Convert.ToInt64( sourceValue) 
                    : (long?) null;
                bool? boolValue = Regex.IsMatch(sourceValue, 
                                  @"^(?i)true$|^(?i)false$")
                    ? Convert.ToBoolean(sourceValue) 
                    : (bool?) null;
                decimal? decimalValue = Regex.IsMatch(sourceValue, 
                                  @"^\d+\.?\d*$")
                    ? Convert.ToDecimal(sourceValue) 
                    : (decimal?) null;
                // multiple if because it didn't like a switch statement
                // I didn't think five micro functions would improve this
                if (targetPropName.PropertyType == typeof(string))
                    targetPropSetter.Invoke(target,
                        new[] { (object) sourceValue });
                if (targetPropName.PropertyType == typeof(Nullable<Int32>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) intValue });
                if (targetPropName.PropertyType == typeof(Nullable<Int64>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) longValue });
                if (targetPropName.PropertyType == typeof(Nullable<Boolean>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) boolValue });
                if (targetPropName.PropertyType == typeof(Nullable<Decimal>))
                    targetPropSetter.Invoke(target,
                        new[] { (object) decimalValue });
            }
        }
    }
}

Answer 1

By seeing the structure of your code structure i assume that all those if staements can easily be changed by a 'switch case' - which obviuosly is a code smell. I think you need to refactor your code. For further details please visit this link

Answer 2

I would consider using TypeDescriptor to help simplify the code. That will allow you to get all the properties and associated TypeConverters without ugly reflection calls. Here is an example:

    public static void CopyProperties(Regex regex, Match match, object target)
    {
        if (regex == null || match == null || target == null)
        {
            throw new ArgumentNullException();
        }

        if (match.Success)
        {
            var properties = TypeDescriptor
                .GetProperties(target)
                .Cast<PropertyDescriptor>()
                .Where(prop => !prop.IsReadOnly);

            foreach (string groupName in regex.GetGroupNames())
            {
                var property = properties
                    .FirstOrDefault(prop => prop.Name == groupName);

                if (property != null && 
                    property.Converter.CanConvertFrom(typeof(string)))
                {
                    try
                    {
                        property.SetValue(
                            target,
                            property.Converter.ConvertFrom(
                                match.Groups[groupName].Value));
                    }
                    catch
                    {
                        // Probably couldn't convert string to type
                    }
                }
            }
        }
    }

The only caveats with the code above is I don't think it will get properties in base classes and the try/catch is a little clunky but it's probably OK if your regex to parse out the groups were specific enough.

Scott Hanselman has a good article about TypeConverters here.