Passing config values as parameters to an instance method C#

Question

I come across this regularly when refactoring code. Say I have a base class and I read some configuration parameters and stuff them into properties like this

public BaseClass()
{
   _property1 = ConfigurationManager.AppSettings["AppSetting1"];
   _property2 = ConfigurationManager.AppSettings["AppSetting2"];
   _property3 = ConfigurationManager.AppSettings["AppSetting3"];
}

And then I call a method in another class like this

OtherClass otherClass = new OtherClass();
var foo = otherClass.SomeMethod(_property1, _property2, _property3);

Is it better to do that? What if I only needed the AppSettings values inside of the OtherClass class? then I could just load them up as private props and initialize them in the constructor and the referencing class/caller wouldn't need to be concerned with the settings.

public OtherClass()
{
   _property1 = ConfigurationManager.AppSettings["AppSetting1"];
   _property2 = ConfigurationManager.AppSettings["AppSetting2"];
   _property3 = ConfigurationManager.AppSettings["AppSetting3"];
}

My implementation would then simply be

OtherClass otherClass = new OtherClass();
var foo = otherClass.SomeMethod();

This one bugs me but I am not really sure why. Which is a better practice and why? And I apologise I am missing something obvious. It happens sometimes lol. Thanks -Frank

Answer 1

I can't really comment on "better" as that's quite subjective, but it's at the very least factual to say that passing the parameters into the method, rather than having the method go and get them itself, is a form of dependency injection. Dependency injection has advantages in that it reduces the number of things the class has to know how to do/reduces the number of other classes any given class needs to do its work. Typically in OO design we look for ways to reduce the dependencies a class has on other classes. You might also see the concept referred to in general as low coupling. Classes that are not highly coupled to other classes are easier to reuse as independent modules within multiple programs

In your example, OtherClass (and/or BaseClass) needs to know what a ConfigurationManager is, which means it needs a reference to its namespace, needs to have system.configuration.dll available on the target etc just so that it can go and get some basic things (strings) that contain info necessary to do its work. If you instead give the strings to the method then it can do its work without knowing what a ConfigurationManager is - you can use it in an app that doesn't even have a ConfigurationManager anywhere, maybe because it gets its config from a database or perhaps it's part of a unit test that gets some contrived data directly from hard coding to ensure a given result is always obtained

When you're down with the concept that the data a class needs to do its work can come from above it starts to make more sense why systems that pass data around like this can work with an inversion-of-control container; essentially software that creates instances of objects for you according to some preconfigured rules about where to get the data that should be passed in. An IoC container can look at an object and decide what arguments to pass to (e.g. its constructor) based on a consistent set of rules, and take another step towards removing dependencies by further reducing use of the word "new". Think of it like writing a config file to describe which of your objects need what instances of other classes to do the work. You craft your IoC container setup so it makes one IniFileConfigSettingsProvider instance and then provides that instance to any object that needs some kind of IConfigSettingsProvider to do its work. Later you switch away form ini files and go to Xml files. You create a class called XmlFileConfigSettingProvider, register it with the IoC and it becomes the new instance that is passed to any class needing an IConfigSettingsProvider. Critically, you made another class, registered it with the IoC and then it gets used throughout your program but you never made an instance of it yourself

If you ever heard the phrase "new is glue" concepts like this are generally what it alludes to - when your OtherClass says var x = new ConfigurationManager... x.Settings["a"].... the use of the word new has suddenly hard wired it to needing a ConfigurationManager; it can't function without knowing what it is. The strive these days is generally to have a class accepting a "passed-in provider of settings that complies with some interface" or "passed-in primitives that are settings" - things that are either implementation specific but obey a generic interface, or ubiquitous in the language and need no special imports respectively. Perhaps either of your mentioned approaches bug you because deep down you feel that neither of them need to depend on ConfigManager; whether they both need settings or not, they can get them passed in, from something higher up the chain that should be making the decisions as to what settings to use

Answer 2

In my view, it depends on what goal of your class.

If class belongs to domain classes, so there is no need to have a dependency to ConfigurationManager class. You can create a constructor and supply necessary data:

public class FooClass()
{
    public Property1 {get; private set;}

    public FooClass(string property1)
    { 
        Property1 = property1;
    }
}

If FooClass belongs to Service Layer, then, in my view, it is eligible to have a dependency to ConfigurationManager class.

Answer 3

There will be pros and cons of every design and coding choice. As they say, same pattern may not fit everyone. So one has to customize based on need.

Mainly, decision should be based on use cases of your application. Let me provide few scenarios to describe it. Suppose items configured in AppSettings will not change in life-time of the your application then you can have an approach in which dependencies with AppSettings are least. In particular an approach as var foo = otherClass.SomeMethod(_property1, _property2, _property3);. This matches with OOD principles as classes will focus on business logic.

But if you see add/modifying/deleting items (even in rare situations) during life time then above approach would be difficult to maintain. For example without restarting your application/WebServer if AppSettings needs to be reloaded based on certain conditions. One may argue why such settings will be kept in AppSettings, which is very valid too. If your application demands such scenarios then it would be better to use ConfigurationManager.AppSettings without worrying about dependencies. One can opt to extend it have wrapper class (Singleton pattern) to manage and provide access to ConfigurationManager.AppSettings.