**Class naming** > I needed a class to assign values to a property of an object this can be reworded to `I needed a class to map values to a property of an object` which results in `IPropertyMapper` and `DynamicPropertyMapper`. ___ **`public void Assign(T target, string propertyName, object value)`** You are doing `null` checks at the top of the method which is very good. Because the `propertyName` will be passed to the `PropertiesMatch()` method which could throw an `AmbiguousMatchException` you should either enclose this call into a `try..catch` or the call to `GetProperty()` inside the `PropertiesMatch()` method. See the [remarks][1] in the docu. You should also state in the xml documentation that the `propertyName` parameter is used case-sensitive. Speaking of xml documentation, you are missing a `to` after `used` > /// Dynamically creates an Action that will be used assign a value to the target's property and adds it to the map. ___ > //This is necessary because we hold a Action<T,object>. var convertedValue = Expression.Convert(valueParameter, propertyType); I love this comment because it is clearly telling **why** you are doing this. ___ In the `CreateAndAddAssignExpression()` method you are adding the `Action<>` as a value to the `Dictionary<>` but you are returning the action as well. Why don't you rename the method to `CreateMappingExpression()`, skip the adding to the dictionary and add the result to the `Dictionary()` where you call it ? For the constructor this would be foreach (var property in properties) { var action = CreateMappingExpression(property.Name, property.PropertyType); _expressionMap.Add(property.Name, action); } and in the `Assign()` method like so if(_lazyPopulate && !_expressionMap.TryGetValue(propertyName, out assignExpression)) { if (PropertiesMatch(value.GetType(), propertyName)) { assignExpression = CreateMappingExpression(propertyName, value.GetType()); } _expressionMap.Add(propertyName, assignExpression); } You see this would remove the `else` part and also makes the comment `//Add null to show we checked that this property doesn't exist in T, so we skip the checks the next time.` redundant. But as [Oguz Ozgul][2] pointed out in his [answer][3] you have a problem if `_lazyPopulate == false` in the way this condition is used. If we revert the condition to check `TryGetValue()` first it won't work without problems but at least without exceptions like so if(!_expressionMap.TryGetValue(propertyName, out assignExpression) && _lazyPopulate) { if (PropertiesMatch(value.GetType(), propertyName)) { assignExpression = CreateMappingExpression(propertyName, value.GetType()); } _expressionMap.Add(propertyName, assignExpression); } [1]: https://msdn.microsoft.com/en-us/library/kz0a8sxy(v=vs.110).aspx#Anchor_2 [2]: http://codereview.stackexchange.com/users/91285/oguz-ozgul [3]: http://codereview.stackexchange.com/questions/113385/dynamic-generic-property-assignator/113465#113465