Tuesday, April 21, 2015

AutoMapper Anti-patterns

I think AutoMapper is evil. It just makes it too easy to do things you should not be doing.

Over the past 4 years I've worked with AutoMapper on several projects and collected a bunch of anti-patterns.

 

Automapper 101

When you see repetitive code appear in a mapping profile this is a sure sign you're not 'doing it right'. For example every mapping from DateTime to string explicitly states that it should use the short date format.

Automapper supports powerful methods of cleaning up the way you map your data. 

Solution

Learn how to use AutoMapper before using it in production code.

Define once how to convert one data type into another (e.g. DateTime to string) so you don't need to repeat that everywhere.

 

Empty profile validation

Create a profile but register every mapping in the global namespace.

Then assert the profile is valid. You are essentially validating an empty profile.

Solution

Don't ever call AutoMapper.CreateMap from an AutoMapper profile, use the instance methods instead.

public class BadProfile: Profile
{
   public override string ProfileName { get { return "BadProfile"; } }
   
   protected override void Configure()
   {
      // !!! Do not do this! It registers the mapping globally !!!
      AutoMapper.CreateMap<DTO.SomeAggregate, Model.SomeAggregate>();
   }
}
public class GoodProfile: Profile
{
   public override string ProfileName { get { return "GoodProfile"; } }
   
   protected override void Configure()
   {
      // Always use the methods on the profile to register mappings
      CreateMap<DTO.SomeAggregat, Model.SomeAggregate>();
   }
}

 

Wait to validate

Credits go to Bastiaan de Rijber for this one.

Asserting profile validity is expensive. If you do this at application startup you may get a serious delay on application startup.

Solution

Assert profile validity in a unit test.

 

Automap all the things

automap-all-the-thingsWhen using automapper is a reflex, it will work against you.

Automapper was intended to reduce repetitive code for mapping types that are highly similar.

If you use it for everything you'll end up with HUGE profiles that are usually more complex than straight up hand-coded mappings.

Solution

Use AutoMapper to get rid of dumb 1-on-1 mappings and simple projections, for example from a primitive type to a localized string.

The rabbit hole

It's easy to build up a mapping profile for a complex data structure like an aggregate root. But keep in mind that when you change your model you will need to untangle the mappings to see which are actually used and where. Since you lost the ability to trace how data is connected due to AutoMappers loose coupling, you'll find yourself in a swamp and sinking fast.

Solution

Keep your mappings simple and your mapping profiles limited in size.

 

Resolvers galore

Building resolvers to do simple mappings is a bad idea. The resolvers add a layer of obscurity to the already loosly coupled mappings. It's all too easy to start hitting the database in these mappings and before you know it you get spanked by N+1 issues.

Solution

If your mapping is more complex than a single expression but it does not involve loading entities create a method for it on the profile or add an extension method that you can reuse. This will prevent obscuring the mapping code in yet another class.

If you do need to load stuff from the database to perform the mapping, seriously reconsider whether AutoMapper is the right tool for the job.

You may be better of coding this by hand using a proper ORM (that's what they are for).

 

'Intelligent' mapping

Resolvers open up a doorway to make your mappings 'intelligent'. This is where resolvers get more and more complexity and soon all sorts of complicated logic starts to creep into the mappings. This smell turns into an outright stink when entities are being created in the resolvers. The net result is that the mappings go from magical to dark voodoo and you find yourself writing unit tests that need to initialize AutoMapper and stub out a bunch of services in order to check the validity of the mapping. A world of hurt.

Solution

Just don't do it. Mapping should always be straight forward.

 

Let me reconfigure that for ya

As mentioned before, setting up mappings is costly. This means you need to think carefully about when you perform that configuration. Once you understand that it’s easy to see that reconfiguring AutoMapper on every, single, call to a service is a bad idea.

Solution

Initialize your mappings once, and once only.

3 comments:

Ibrahim ben Salah said...

I think AutoMapper is worse than evil. I think you implies that AutoMapper provides loose coupling, but do we really need it when most of transformations are non trivial, Provided as proof the long list of mappings, back and forth, with so much ignores and exceptions?

benk said...

Agree ..

I can see some side cases like end to end tests where you need to map to lots of types and a lot of mvc views but if you have 1 bug / issue with auto mapper and I have had many or you need to talk about the issue than its not worth it. Issues I have had/ seen

Complex initialization
bad policy based conversions eg enums
Datetime conversion with datetime kind.
Multi threaded initialization issue
Unit tests issue due to initialization
Performance.
Child objects that should be null and receiver type that should not.
Filling the reflection hot 16 type cache.

Also agree on the bad pattern - you get lasagna / anemic architectures and its deceptive when you start often you have a DTO and an internal object being the same however to keep auto mapper happy you don't make changes you should to types as that means custom config. If all the objects are the same you may as well use the same object and when you need to change it then change it ..



Just write some code and use symantic comparison in tests to make sure its correct .

Unknown said...

AutoMappper create brittle code and hard to debug code because often you'll discover the issue at runtime rather than compile time. I just ran into one situation where I had two converters defined using ConvertUsing() and threw an obtuse exception on the 2nd converter. However using ConvertUsing() for the first converter then using ConvertUsing(new T()) worked. It took me hours to find a workaround (anti-pattern) since using ConvertUsing() intuitively should have worked for both.

Often time I just say why not use an extension method and pass your source and destination. It's much easier and errors are detected at compile time. AutoMapper overkill and add too much complexity to your code.