Dissecting AutoMapper Programming Horror

My AutoMapper search alerts brought me this gem this morning:

Hey... let's use automapper to generate passwords.

This post illustrates basically all the wrong reasons to use AutoMapper. Here's the code minus the unimportant bits:

public class RequestProfile : Profile
{
    public RequestProfile()
    {
        CreateMap<Request, NewAccount>()
            .ForMember(d => d.Address, e => e.MapFrom(request => request))
            .ForMember(d => d.Salutation, e => e.MapFrom(request => request.Title))
            .ForMember(d => d.FirstName, e => e.MapFrom(request => request.FirstName))
            .ForMember(d => d.MiddleName, e => e.MapFrom(request => request.MiddleName))
            .ForMember(d => d.LastName, e => e.MapFrom(request => request.LastName))
            .ForMember(d => d.Email, e => e.MapFrom(request => request.Email))
            .ForMember(d => d.Password, e => e.MapFrom(_ => "Password1!" + DateTime.Now))
            .ForMember(d => d.CommunicationOptIn, e => e.MapFrom(request => request.AgreeNewsLetter))
            .ForMember(d => d.DateOfBirth, e => e.MapFrom(request => request.DateOfBirth))
            .ForMember(d => d.Id, e => e.Ignore())
            .ForMember(d => d.AccountType, e => e.Ignore());
}

The highlighted horror was the Password configuration, but everything else is wrong about this code as well. So much, I'll take it line by line.

First up:

ForMember(d => d.Address, e => e.MapFrom(request => request))

If this code seems confusing, that's because it is. This configuration says "for this child member Address, map from the parent object". This typically happens when a source type looks like a flattened version of the destination type, except the prefix member names aren't there. Instead of:

public class Request
{
    public string AddressLine1 { get; set; }
    public string AddressLine2 { get; set; }
    public string AddressCity { get; set; }
    public string AddressState { get; set; }
    public string AddressZip { get; set; }
}

public class NewAccount
{
    public Address Address { get; set; }
}

public class Address
{
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string City { get; set; }
    public string State { get; set; }
    public string Zip { get; set; }
}

It's likely:

public class Request
{
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string City { get; set; }
    public string State { get; set; }
    public string Zip { get; set; }
}

public class NewAccount
{
    public Address Address { get; set; }
}

public class Address
{
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string City { get; set; }
    public string State { get; set; }
    public string Zip { get; set; }
}

And because the names no longer match up exactly and the types were flattened without the prefix e.g. Address.Line1 <-> AddressLine1, we now have this...confusing configuration.

Next, we have:

.ForMember(d => d.FirstName, e => e.MapFrom(request => request.FirstName))
.ForMember(d => d.MiddleName, e => e.MapFrom(request => request.MiddleName))
.ForMember(d => d.LastName, e => e.MapFrom(request => request.LastName))
.ForMember(d => d.Email, e => e.MapFrom(request => request.Email))
.ForMember(d => d.DateOfBirth, e => e.MapFrom(request => request.DateOfBirth))

This configuration is redundant. AutoMapper already lines up these values. But I see folks do this in cases where maps don't really line up and they'd rather "be explicit". Don't. If you have to explicitly configure what is implicit, it's not "Auto" anymore, and not an appropriate use case of AutoMapper.

Finally, all the manual configuration:

.ForMember(d => d.Salutation, e => e.MapFrom(request => request.Title))
.ForMember(d => d.Password, e => e.MapFrom(_ => "Password1!" + DateTime.Now))
.ForMember(d => d.CommunicationOptIn, e => e.MapFrom(request => request.AgreeNewsLetter))
.ForMember(d => d.Id, e => e.Ignore())
.ForMember(d => d.AccountType, e => e.Ignore())

So of the 12-16 source/destination values (including the Address business), only 5 match. I recommend folks CONSIDER using AutoMapper when it's dumb flattening, you want to enforce a convention and 95% of everything lines up. This does not match any of that. I'd replace their usage of AutoMapper of that config and:

var newAccount = mapper.Map<Request, NewAccount>(request);

with:

var newAccount = new NewAccount
{
    Address = new Address
    {
        Line1 = request.Line1,
        Line2 = request.Line2,
        City = request.City,
        State = request.State,
        Zip = request.Zip
    },
    Salutation = request.Title,
    FirstName = request.FirstName,
    MiddleName = request.MiddleName,
    LastName = request.LastName,
    Email = request.Email,
    Password = "Password1!" + DateTime.Now, //yes this is hot garbage
    CommunicationOptIn = request.AgreeNewsLetter,
    DateOfBirth = request.DateOfBirth
};

Much more obvious and explicit. If your AutoMapper configuration looks like the above, remove AutoMapper and replace with code.