views:

821

answers:

4

I am configuring Automapper in the Bootstrapper and I call the Bootstrap() in the Application_Start(), and I've been told that this is wrong because I have to modify my Bootstrapper class each time I have to add a new mapping, so I am violating the Open-Closed Principle.

How do you think, do I really violate this principle?

public static class Bootstrapper
{
    public static void BootStrap()
    {
        ModelBinders.Binders.DefaultBinder = new MyModelBinder();
        InputBuilder.BootStrap();
        ConfigureAutoMapper();
    }

    public static void ConfigureAutoMapper()
    {
        Mapper.CreateMap<User, UserDisplay>()
            .ForMember(o => o.UserRolesDescription,
                       opt => opt.ResolveUsing<RoleValueResolver>());
        Mapper.CreateMap<Organisation, OrganisationDisplay>();
        Mapper.CreateMap<Organisation, OrganisationOpenDisplay>();
        Mapper.CreateMap<OrganisationAddress, OrganisationAddressDisplay>();
    }    
}
+3  A: 

To have it completely closed, you could have a static initializer per Mapping registration, but that would be overkill.

Some things are actually useful to have centralised to a degree from the point of view of being able to reverse engineer though.

In NInject, there is the notion of having a Module per project or subsystem (set of projects), which seems a sensible compromise.

Ruben Bartelink
+2  A: 

If anything its the single responsibility principle that you are violating, in that the class has more than one reason to change.

I personally would have a ConfigureAutoMapper class which all my configuration for AutoMapper was done with. But it could be argued that it is down to personal choice.

Kev Hunter
yes, and still, after I move it to another class, I don't get away from the Open Closed Principle
Omu
+2  A: 

Omu, I wrestle with similar questions when it comes to bootstrapping an IoC container in my app's startup routine. For IoC, the guidance I've been given points to the advantage of centralizing your configuration rather than sprinkling it all over your app as you add changes. For configuring AutoMapper, I think the advantage of centralization is much less important. If you can get your AutoMapper container into your IoC container or Service Locator, I agree with Ruben Bartelink's suggestion of configuring the mappings once per assembly or in static constructors or something decentralized.

Basically, I see it as a matter of deciding whether you want to centralize the bootstrapping or decentralize it. If you are that concerned about the Open/Closed Principle on your startup routine, go with decentralizing it. But your adherence to OCP can be dialed down in exchange for the value of all your bootstrapping done in one place. Another option would be to have the bootstrapper scan certain assemblies for registries, assuming AutoMapper has such a concept.

flipdoubt
+9  A: 

I would argue that you are violating two principles: the single responsibility principle (SRP) and the open/closed principle (OCP).

You are violating the SRP because the bootstrapping class have more than one reason to change: if you alter model binding or the auto mapper configuration.

You would be violating the OCP if you were to add additional bootstrapping code for configuring another sub-component of the system.

How I usually handle this is that I define the following interface.

public interface IGlobalConfiguration
{
    void Configure();
}

For each component in the system that needs bootstrapping I would create a class that implements that interface.

public class AutoMapperGlobalConfiguration : IGlobalConfiguration
{
    private readonly IConfiguration configuration;

    public AutoMapperGlobalConfiguration(IConfiguration configuration)
    {
        this.configuration = configuration;
    }

    public void Configure()
    {
        // Add AutoMapper configuration here.
    }
}

public class ModelBindersGlobalConfiguration : IGlobalConfiguration
{
    private readonly ModelBinderDictionary binders;

    public ModelBindersGlobalConfiguration(ModelBinderDictionary binders)
    {
        this.binders = binders;
    }

    public void Configure()
    {
        // Add model binding configuration here.
    }
}

I use Ninject to inject the dependencies. IConfiguration is the underlying implementation of the static AutoMapper class and ModelBinderDictionary is the ModelBinders.Binder object. I would then define a NinjectModule that would scan the specified assembly for any class that implements the IGlobalConfiguration interface and add those classes to a composite.

public class GlobalConfigurationModule : NinjectModule
{
    private readonly Assembly assembly;

    public GlobalConfigurationModule() 
        : this(Assembly.GetExecutingAssembly()) { }

    public GlobalConfigurationModule(Assembly assembly)
    {
        this.assembly = assembly;
    }

    public override void Load()
    {
        GlobalConfigurationComposite composite = 
            new GlobalConfigurationComposite();

        IEnumerable<Type> types = 
            assembly.GetExportedTypes().GetTypeOf<IGlobalConfiguration>()
                .SkipAnyTypeOf<IComposite<IGlobalConfiguration>>();

        foreach (var type in types)
        {
            IGlobalConfiguration configuration = 
                (IGlobalConfiguration)Kernel.Get(type);
            composite.Add(configuration);
        }

        Bind<IGlobalConfiguration>().ToConstant(composite);
    }
}

I would then add the following code to the Global.asax file.

public class MvcApplication : HttpApplication
{
    public void Application_Start()
    {
        IKernel kernel = new StandardKernel(
            new AutoMapperModule(),
            new MvcModule(),
            new GlobalConfigurationModule()
        );

        Kernel.Get<IGlobalConfiguration>().Configure();
    }
}

Now my bootstrapping code adheres to both SRP and OCP. I can easily add additional bootstrapping code by creating a class that implements the IGlobalConfiguration interface and my global configuration classes only have one reason to change.

mrydengren
and you still have to change the Configure method in AutoMapperGlobalConfiguration each time you need to add a new mapping
Omu
But that would not violate OCP. OCP is not write once never touch again. OCP states that the consumer of the bootstrapping code, the GlobalConfigurationModule (GCM), should rely on abstraction and not conrete implementation. If I were to add bootstrapping for log4net I would create a class Log4NetGlobalConfiguration class that would implement IGlobalConfiguration. However, I would not have to modify any other part of my code and definitely not the GCM because it has no intricate knowledge of the conrete implementation of the IGlobalConfiguration interface.
mrydengren