Dynamic, generic property "assignator"

Point one, I know "Assignator" isn't a real word. I couldn't find a better fit so right now that word is added to my Visual Studio's dictionary!

As a part of my (future) MVVM framework, I needed a class to assign values to a property of an object. As it's a quite generic framework, the class itself needed to be generic. I had the choice to use reflection (booo) or to give it a try and Expression compilation at runtime (cheers)! Guess what, I went with expression compilation.

```
public interface IPropertyAssignator where T : class
{
void Assign(T target, string propertyName, object value);
}

///
/// Dynamically assigns values to properties of an object
///
/// Assignation's target type
/// This class is not thread safe.
public class DynamicPropertyAssignator : IPropertyAssignator where T : class
{
private readonly bool _lazyPopulate;
private readonly Dictionary> _expressionMap;

///
/// Creates an instance of DynamicPropertyAssignator.
///
/// If true, assignation expressions will be generated only when needed.
public DynamicPropertyAssignator(bool lazyPopulate = true)
{
_lazyPopulate = lazyPopulate;
_expressionMap = new Dictionary>();

if (!lazyPopulate)
{
var properties = typeof(T).GetProperties(BindingFlags.Instance | BindingFlags.Public);

foreach (var property in properties)
{
CreateAndAddAssignExpression(property.Name, property.PropertyType);
}
}
}

///
/// Assigns to of
///
/// Target to be modified
/// /// Property of the target to modify
/// Value to assign the target
/// If the assignation arguments are of incoherent types.
public void Assign(T target, string propertyName, object value)
{
if (target == null) throw new ArgumentNullException(nameof(target));
if (propertyName == null) throw new ArgumentNullException(nameof(propertyName));
if

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 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.
  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 pointed out in his answer 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);
     }

StackExchange Code Review Q#113385, answer score: 6