3
\$\begingroup\$

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 });
            }
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

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

\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

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

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