Enhancement: Convert DefaultTypeMap to a Strategy

[GoogleCodeExporter]

I have a use case where I am working solely with stored procedures.   Most of 
the time, the POCO properties are going to need to be renamed to adhere to C# 
coding/naming standards.   Much of this will be done via T4 template code 
generation.

The problem is that nearly every type needs to be added with a custom type 
mapper.  It's a lot of boiler plate code.

What I would like to do is be able to replace the DefaultTypeMap implementation 
such that attributes (hopefully something from the BCL) on each POCO will be 
used without having to add a type mapper for each POCO.

I think its a minimal amount of code to add, like below:

public interface ITypeMapStrategy{
   ITypeMap CreateTypeMap(Type type);
}

private static _typeMapStrategy = new DefaultTypeMapStrategy();
public static TypeMapStrategy
{
    get
    {
      return _typeMapStrategy;
    }
    set
    {
      _typeMapStrategy=value;
    }
}

public DefaultTypeMapStrategy : ITypeMapStrategy
{
    public ITypeMap CreateTypeMap(Type type)
    {
        return new DefaultTypeMap(type);
    }
}

replace calls to:
  new DefaultTypeMap(type)
with:
  TypeMapStrategy.CreateTypeMap(type)

Original issue reported on code.google.com by richard....@gmail.com on 11 Jun 2013 at 3:58

[GoogleCodeExporter]

Sorry the line:
private static _typeMapStrategy = new DefaultTypeMapStrategy();

should be 
private static ITypeMapStrategy _typeMapStrategy = new DefaultTypeMapStrategy();

Original comment by richard....@gmail.com on 11 Jun 2013 at 4:02

[GoogleCodeExporter]

This definitely has potential

Original comment by marc.gravell on 12 Jun 2013 at 6:40

• Changed state: Accepted

[GoogleCodeExporter]

This would be very helpful for me.

Original comment by cklep...@gmail.com on 11 Sep 2013 at 6:27

[GoogleCodeExporter]

Thinking out loud here, I am not sure a static would be a good idea on second 
thought.  You would be restricted to a  single type mapping strategy in an app 
domain.  In theory, your application could reference two different DAL 
assemblies, each making assumptions about the type mapping strategy without 
regard for the other.   I believe they would be stepping on each other's toes.

I am not sure what other option there is though since the Dapper extension 
methods are essentially all static.   I guess implementing as a static is 
acceptable with the limitations acknowledged.  

The only other thing I can think of is to include the mapping strategy as a 
parameter of the extension method.

Original comment by richard....@gmail.com on 11 Sep 2013 at 9:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Here's what I believe should be the order of precedence for the mapping:

1. type mapping - currently in dapper (map = (ITypeMap)_typeMaps[type];)
2. specified type mapping strategy (specified as a Guid in the Query function, 
defaulted to null)
3. default type mapping strategy
4. Dapper default type mapping (map = new DefaultTypeMap(type);)

Here's some rough code:

public interface ITypeMapStrategy
{
    Guid StrategyIdentifier { get; }
    ITypeMap CreateTypeMap(Type type);
}

private static Guid defaultTypeMapStrategyKey;

private static ITypeMapStrategy defaultTypeMapStrategy
{
    get { return (typeMapStrategies == null) ? null : typeMapStrategies[defaultTypeMapStrategyKey]; }
}

private static IDictionary<Guid, ITypeMapStrategy> typeMapStrategies;

public static void AddTypeMapStrategy(ITypeMapStrategy typeMapStrategy, bool 
isDefault = false)
{
    if (typeMapStrategies == null)
        typeMapStrategies = new Dictionary<Guid, ITypeMapStrategy>();

    if (typeMapStrategies.ContainsKey(typeMapStrategy.StrategyIdentifier))
        throw new InvalidOperationException("Strategy for supplied StrategyIdentifier already defined");

    if (isDefault)
    {
        if(defaultTypeMapStrategyKey != null)
            throw new InvalidOperationException("Default type map strategy already defined");
        else
            defaultTypeMapStrategyKey = typeMapStrategy.StrategyIdentifier;
    }

    typeMapStrategies.Add(typeMapStrategy.StrategyIdentifier, typeMapStrategy);
}

In GetTypeMap(Type type) it would look something like this:

map = (ITypeMap)_typeMaps[type];

if (specificTypeStrategy != null)
    map = typeMapStrategies[(Guid)specificTypeStrategy].CreateTypeMap(type);

if (map == null && defaultTypeMapStrategy != null)
    map = defaultTypeMapStrategy.CreateTypeMap(type);

if (map == null)
{
    map = new DefaultTypeMap(type);
    _typeMaps[type] = map;
}

specificTypeStrategy above would be a Guid? argument in GetTypeMap. Query would 
need a Guid? added as well to define a specific strategy (defaulted to null).

Original comment by cklep...@gmail.com on 12 Sep 2013 at 2:59

[GoogleCodeExporter]

In GetTypeMap I have a logical error:

if (specificTypeStrategy != null)   
should be 
if (map == null && specificTypeStrategy != null)

The strategy identifier doesn't have to be a guid, it could be a string or int 
so long as its unique to the dictionary.

We'd probably want to store the generated typemap from the call to 
CreateTypeMap in some sort of static collection (HashTable like dapper uses?) 
to pull it quicker after subsequent calls to this function.

Original comment by cklep...@gmail.com on 12 Sep 2013 at 12:38

[GoogleCodeExporter]

I am going to play devils advocate on the dictionary idea.  This introduces 
some overhead in doing null checking and look up.   If the the client code has 
a reference to the type mapping strategy it wants to use and passes that in to 
the extension method, there are no look ups, comparisons, etc. to be performed 
and performance is unaffected.  For example, assembly A might have a 
static/singleton instance of a type mapping strategy and assembly B would do 
the same.   There would be no performance impact in this case.

If the client needs to keep a dictionary of strategies, then that is fine too 
but it doesn't impose that performance hit on all users of dapper.

In applications that I normally deal with a dictionary would not be a concern.  
But I am thinking with something like dapper which is being used with high 
traffic sites, performance is everything.

Original comment by richard....@gmail.com on 12 Sep 2013 at 1:05

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I agree, the code above certainly is not optimized. The goal being to add this 
functionality without affecting performance.

Personally, we would be happy with just having 1 default strategy defined 
without needing multiple strategies (like you outlined in the original post). 
If you use multiple DAL's then you can place your switching logic in your 
implementation of CreateTypeMap. Optimization would be on the client end in 
that regard.

Original comment by cklep...@gmail.com on 12 Sep 2013 at 1:52