Is AutoMapper a Code Smell?
By Harry Bellamy
- 4 minutes read - 692 wordsI’d like to get this out of the way before I get into my thoughts on this - this is in no way a dig a Jimmy Bogard. He has provided a clear set of guidelines for the usage here.
Unfortunately in my experience, many projects don’t follow these guidelines (and don’t get me wrong I’m guilty of violating some in the past myself).
Abstracting IMapper
This I find strange. As IMapper
is injected into classes, the temptation is to mock it. However, the mapping is often (you could argue always) intrinsic to the business logic under test.
So why abstract it? Tight coupling feels acceptable here as it feels very unlikely that an alternative implementation of IMapper
would be provided.
Who needs tests, it’s just mapping
In my experience, the mapping layer is often not covered by the unit tests. This may be due to it being embedded inside the application startup code making it difficult or impossible to test.
Separating all your mapping out into a profile is a good start, even if it is monolithic. If you are working on an unfamiliar codebase you may not have the relevant knowledge to divide it up any further.
If you have multiple profiles, consider creating an aggregated configuration that you can test to flag any duplicate mappings:
using AutoMapper;
public class MyMapperConfiguration : MapperConfiguration
{
public MyMapperConfiguration()
: base(cfg =>
{
cfg.AddProfile(new Profile1());
cfg.AddProfile(new Profile2());
})
{ }
}
This means you can then create a test to run AssertConfigurationIsValid()
on this configuration.
Warning: this can flag up some scary issues once run that may not be easy to remediate. Fixing these issues may necessitate regression testing in the affected areas.
AutoMapper usage often hides code coverage
OK, so you’ve setup unit tests that call AssertConfigurationIsValid()
on each of your profiles and on your aggregated configuration.
That means all your mappings are correct, right??
Sadly, no.
All AssertConfigurationIsValid()
does is ensure that each property on the destination type is mapped to or ignored.
However, the code that creates the mappings has been run in one or more tests so is deemed covered. This gives a false sense of security as it’s not been functionality tested.
So what’s the solution? Create a unit test for every mapping? This could be very time consuming, especially on an unfamiliar codebase.
What else could I use?
On a brand new codebase I probably wouldn’t use AutoMapper at all. The overhead of policing it to make sure it’s being used properly is hard to justify in my opinion. Another bonus is it’s one fewer dependency to maintain.
I would consider using one of two native C# approaches or a combination of them. Both these approaches make increasing amounts of sense when dealing with complex object graphs. Both of these approaches also have the advantage of being debuggable, in contrast to the mystery of what goes on under the hood of AutoMapper.
1. Using LINQ Select to perform a projection on enumerables
LINQ Select()
performs a projection which is just a mapping if you think about it (not too hard).
public IEnumerable<ResultType> GetData()
{
// Get the source data somehow
...
return sourceData
.Select(s => new ResultType
{
Property1 = s.Prop1,
Property2 = s.Prop2
});
}
You can use immutable types if you’d rather here (admittedly AutoMapper supports immutable types as well).
2. Write extension methods to perform mapping
Ah extension methods. Those who work with me know my love of extension methods, but I will wax lyrical in a future post.
An extension method implementation could look something like this:
public static ResultType MapToResultType(this SourceType sourceType)
=> new ResultType
{
Property1 = sourceType.Prop1,
Property2 = sourceType.Prop2
});
Advantages of this approach are that you can test it in isolation from any implementation and it’s reusable (yes, like an AutoMapper mapping…).
The disadvantage of both these approaches is they are more verbose, but such explicit implementations are easier to debug and (in my opinion) easier for other developers to extract the meaning of your code.
TL;DR
Make sure your mapping layers are tested. Consider pivoting away from AutoMapper in favour of explicit mapping code where possible.