Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I am working on a helper method that will map properties from an ExpandoObject to a user supplied object and was wondering if the code could be cleaned up or made any more efficiently. It currently has the correct behaviour from a simple test.

public static class Mapper
{
    public static void Map<T>(ExpandoObject source, T destination)
    {
        IDictionary<string, object> dict = source;
        var type = destination.GetType();

        foreach (var prop in type.GetProperties())
        {
            var lower = prop.Name.ToLower();
            var key = dict.Keys.SingleOrDefault(k => k.ToLower() == lower);

            if (key != null)
            {
                prop.SetValue(destination, dict[key], null);
            }
        }
    }
}

Full test can be seen here. There is currently no type checking. Would this be next to add?

share|improve this question
up vote 7 down vote accepted

I've come up with a few changes that should actually speed it up.

// By using a generic class we can take advantage
// of the fact that .NET will create a new generic type
// for each type T. This allows us to avoid creating
// a dictionary of Dictionary<string, PropertyInfo>
// for each type T. We also avoid the need for the 
// lock statement with every call to Map.
public static class Mapper<T>
    // We can only use reference types
    where T : class
{
    private static readonly Dictionary<string, PropertyInfo> _propertyMap;

    static Mapper()
    {
        // At this point we can convert each
        // property name to lower case so we avoid 
        // creating a new string more than once.
        _propertyMap = 
            typeof(T)
            .GetProperties()
            .ToDictionary(
                p => p.Name.ToLower(), 
                p => p
            );
    }

    public static void Map(ExpandoObject source, T destination)
    {
        // Might as well take care of null references early.
        if (source == null)
            throw new ArgumentNullException("source");
        if (destination == null)
            throw new ArgumentNullException("destination");

        // By iterating the KeyValuePair<string, object> of
        // source we can avoid manually searching the keys of
        // source as we see in your original code.
        foreach (var kv in source)
        {
            PropertyInfo p;
            if (_propertyMap.TryGetValue(kv.Key.ToLower(), out p))
            {
                var propType = p.PropertyType;
                if (kv.Value == null)
                {
                    if (!propType.IsByRef && propType.Name != "Nullable`1")
                    {
                        // Throw if type is a value type 
                        // but not Nullable<>
                        throw new ArgumentException("not nullable");
                    }
                }
                else if (kv.Value.GetType() != propType)
                {
                    // You could make this a bit less strict 
                    // but I don't recommend it.
                    throw new ArgumentException("type mismatch");
                }
                p.SetValue(destination, kv.Value, null);
            }
        }
    }
}
share|improve this answer
    
I'd advise you to check out Fasterflect (fasterflect.codeplex.com), which is a library to make reflection tasks easier (through an abstraction over the classic reflection API) and much faster (using dynamic code generation). Also worth mentioning is AutoMapper, which is a library dedicated just to the task of mapping between objects. – Morten Mertner Mar 2 '11 at 14:28
    
May be ok for now but needs more work to handle nested types. – nawfal Jul 19 '14 at 8:59

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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