Dependency Injection and AppSettings
Asked Answered
G

2

11

Let's say I am defining a browser implementation class for my application:

class InternetExplorerBrowser : IBrowser {
    private readonly string executablePath = @"C:\Program Files\...\...\ie.exe";

    ...code that uses executablePath
}

This might at first glance to look like a good idea, as the executablePath data is near the code that will use it.

The problem comes when I try to run this same application on my other computer, that has a foreign-language OS: executablePath will have a different value.

I could solve this through an AppSettings singleton class (or one of its equivalents) but then no-one knows my class is actually dependent on this AppSettings class (which goes against DI ideias). It might pose a difficulty to Unit-Testing, too.

I could solve both problems by having executablePath being passed in through the constructor:

class InternetExplorerBrowser : IBrowser {
    private readonly string executablePath;

    public InternetExplorerBrowser(string executablePath) {
        this.executablePath = executablePath;
    }
}

but this will raise problems in my Composition Root (the startup method that will do all the needed classes wiring) as then that method has to know both how to wire things up and has to know all these little settings data:

class CompositionRoot {
    public void Run() {
        ClassA classA = new ClassA();

        string ieSetting1 = "C:\asdapo\poka\poskdaposka.exe";
        string ieSetting2 = "IE_SETTING_ABC";
        string ieSetting3 = "lol.bmp";

        ClassB classB = new ClassB(ieSetting1);
        ClassC classC = new ClassC(B, ieSetting2, ieSetting3);

        ...
    }
}

which will turn easily a big mess.

I could turn this problem around by instead passing an interface of the form

interface IAppSettings {
    object GetData(string name);
}

to all the classes that need some sort of settings. Then I could either implement this either as a regular class with all the settings embedded in it or a class that reads data off a XML file, something along the lines. If doing this, should I have a general AppSettings class instance for the whole system, or have an AppSettings class associated to each class that might need one? That certainly seems like a bit of an overkill. Also, have all the application setings in the same place makes it easy to look and see what might be all the changes I need to do when tryign to move the program to different platforms.

What might be the best way to approach this common situation?

Edit:

And what about using an IAppSettings with all its settings hardcoded in it?

interface IAppSettings {
    string IE_ExecutablePath { get; }
    int IE_Version { get; }
    ...
}

This would allow for compile-time type-safety. If I saw the interface/concrete classes grow too much I could create other smaller interfaces of the form IMyClassXAppSettings. Would it be a burden too heavy to bear in med/big sized projects?

I've also reading about AOP and its advantages dealing with cross-cutting-concerns (I guess this is one). Couldn't it also offer solutions to this problem? Maybe tagging variables like this:

class InternetExplorerBrowser : IBrowser {
    [AppSetting] string executablePath;
    [AppSetting] int ieVersion;

    ...code that uses executablePath
}

Then, when compiling the project we'd also have compile time safety (having the compiler check that we actually implemented code that would weave in data. This would, of course, tie our API to this particular Aspect.

Gerardgerardo answered 27/8, 2010 at 19:10 Comment(3)
What's your objection to putting it in your composition root? Normally a Main method or an Application_Start routine is the piece of code closest to and most knowledgeable about the configuration mechanism, so it's the most sensible place to put configuration logic, whether that involves parsing command-line arguments, reading a configuration file, or obtaining information about the execution context.Gilburt
I only have an objection against passing strings, ints, etc in the object's constructors, as then all the startup code gets a big mess. On the other hand, having the startup just instantiate a AppSettings class and passing it to all classes that need it seems alright to me.Gerardgerardo
What is the part which is not?Gerardgerardo
S
15

The individual classes should be as free from infrastructure as possible - constructs like IAppSettings, IMyClassXAppSettings, and [AppSetting] bleed composition details to classes which, at their simplest, really only depend on raw values such as executablePath. The art of Dependency Injection is in the factoring of concerns.

I have implemented this exact pattern using Autofac, which has modules similar to Ninject and should result in similar code (I realize the question doesn't mention Ninject, but the OP does in a comment).

Modules organize applications by subsystem. A module exposes a subsystem's configurable elements:

public class BrowserModule : Module
{
    private readonly string _executablePath;

    public BrowserModule(string executablePath)
    {
        _executablePath = executablePath;
    }

    public override void Load(ContainerBuilder builder)
    {
        builder
            .Register(c => new InternetExplorerBrowser(_executablePath))
            .As<IBrowser>()
            .InstancePerDependency();
    }
}

This leaves the composition root with the same problem: it must supply the value of executablePath. To avoid the configuration soup, we can write a self-contained module which reads configuration settings and passes them to BrowserModule:

public class ConfiguredBrowserModule : Module
{
    public override void Load(ContainerBuilder builder)
    {
        var executablePath = ConfigurationManager.AppSettings["ExecutablePath"];

        builder.RegisterModule(new BrowserModule(executablePath));
    }
}

You could consider using a custom configuration section instead of AppSettings; the changes would be localized to the module:

public class BrowserSection : ConfigurationSection
{
    [ConfigurationProperty("executablePath")]
    public string ExecutablePath
    {
        get { return (string) this["executablePath"]; }
        set { this["executablePath"] = value; }
    }
}

public class ConfiguredBrowserModule : Module
{
    public override void Load(ContainerBuilder builder)
    {
        var section = (BrowserSection) ConfigurationManager.GetSection("myApp.browser");

        if(section == null)
        {
            section = new BrowserSection();
        }

        builder.RegisterModule(new BrowserModule(section.ExecutablePath));
    }
}

This is a nice pattern because each subsystem has an independent configuration which gets read in a single place. The only benefit here is a more obvious intent. For non-string values or complex schemas, though, we can let System.Configuration do the heavy lifting.

Sapper answered 28/8, 2010 at 20:40 Comment(2)
builder.Register(c => new InternetExplorerBrowser(_executablePath)) I like your idea providing the single configuration options instead of the whole configuration object. However, how do you implement this, if a class needs a mix of configuration options (usually primitive types) and services (complex objects): class InternetExplorerBrowser(string path, IOtherService service) ```` IOtherService```` should be resolved by the IoC container, but path manually from my config. Solutions like https://mcmap.net/q/50169/-creating-an-instance-using-ninject-with-additional-parameters-in-the-constructor require still more boilerplate codeCement
@NiklasPeter: In the case of Autofac, the c argument represents the resolution context, which you can ask for services: Register(c => new InternetExplorerBrowser(_executablePath, c.Resolve<IOtherService>()));.Sapper
C
2

I'd go with the last option - pass in an object that complies with the IAppSettings interface. In fact, I recently performed that refactor at work in order to sort out some unit tests and it worked nicely. However, there were few classes dependent on the settings in that project.

I'd go with creating a single instance of the settings class, and pass that in to anything that's dependant upon it. I can't see any fundamental problem with that.

However, I think you've already thought about this and seen how it can be a pain if you have lots of classes dependent on the settings.

If this is a problem for you, you can work around it by using a dependency injection framework such as ninject (sorry if you're already aware of projects like ninject - this might sound a bit patronizing - if you're unfamiliar, the why use ninject sections on github are a good place to learn).

Using ninject, for your main project you can declare that you want any class with a dependency on IAppSettings to use a singleton instance of your AppSettings based class without having to explicitly pass it in to constructors everywhere.

You can then setup your system differently for your unit tests by stating that you want to use an instance of MockAppSettings wherever IAppSettings is used, or by simply explicitly passing in your mock objects directly.

I hope I've got the gist of your question right and that I've helped - you already sound like you know what you're doing :)

Cangue answered 28/8, 2010 at 11:23 Comment(2)
Hello. I am well aware both of DI frameworks and about Ninject(I have been using it, in fact!). I largely agree with your point of view. I've updated my OP. Have a look at it, if possible!Gerardgerardo
@devoured elysium - After reading through @Bryan's answer, it does seem a little bit like my chosen solution is a bit DI-happy, and I think I 'get' your question a bit more now. I forget that DI is main uses come in when a class needs to consume 'services'. A class representing some settings doesn't seem much like a service to me (unless it supports things like change notification). @Bryan's answer seems like a good suggestion which will help keep the majority of your settings-dependent classes nice and simple.Cangue

© 2022 - 2024 — McMap. All rights reserved.