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.