Global / Application settings and testing

Question

We have a global dictionary of application settings (read from a DB on start-up and refreshed when required) and a static class with a bunch of methods to get these settings. This is historic, but I see no reason why it would be done differently today unless it adds value in testing...

The options for swapping out values for testing different scenarios I think include:

1. Updating the database values before the test
2. Mock the static methods and intercept the ones you need and allow defaults for the ones that don't impact the test
3. Refactor code to take in a settings instance (e.g. IDepartmentApiSettings) at construction time (constructor injection) and use this instead of the static methods

The first two work without code change, so are preferred on the legacy code. The third therefore would only be viable for new code (or code that can be easily refactored).

Now here's my question, is there any real advantage to injecting the settings in this way? Or would you live with static methods and use option 1 or 2 to keep consistency across the code base?

Note: Code is C# but that shouldn't make any difference, would be the same question for Java, so I won't tag it.

Answer 1

Personally, I would not like to live with static methods to keep consistency across a code base.

The advantages, whether you consider them "real" or not, of injecting the settings lie, as always in the freedom gained and the disadvantages avoided.

I'd always prefer to inject a settings instance as it ties you down the least and doesn't force you to use a mocking framework.

Mocking the static methods is the next best, I guess. As you are in C# and static and virtual don't mix there, I can't think of a convenient way - like using a test descendant and overriding the methods - to mock the methods without using a mocking framework.

Updating the database values before the test is horrible. As your tests then have an external dependency, they automatically become integration tests. It can work, but either you have to make your tests all use the same values or update the values every test method execution. The first is horribly restrictive. The latter will slow down your tests extremely and make everybody turn them off... No use having them in that case.

In short: injection prefered, use mocking the static methods as a crutch if you can't afford to update all uses of your static settings class to an injection pattern all in one go.

If I understand your setup with the settings class correctly, another option might be to keep your static class, make reading the values from the database happen as part of an initialization call on that class. Give the initialization method two parameters: one name/value pairs or whatever you like that you can pass in from your tests to populate your settings dictionary, the other to indicate whether values should be read from the database. Make the defaults to be an empty list of name/value pairs and "yes" read from db. (You may need to use overloads in C# to allow for default parameter values).

Now you also have your hands free for testing without affecting production use of the static class.

• Production code would simply call Initialize without using any parameters at some point in the startup of your application.
• Test code can call with an empty list and "no" to clear the settings.
• Test code can call with a list of name/value pairs and "no" to provide a limited set of specific values (clearing out what was there before if you so desire).
• Test code can call with a list of name/value pairs and "yes" to provide overrides to whatever will be read from the database.

Answer 2

What you describe is a thinly-veiled coating over Singleton. Essentially, you have global state and are trying to funnel access through that one class. The fact that the interface is static instead of object-owned is the "coating" I referred to.

This is one of the few acceptable uses for Singleton. While that pattern is often misapplied, providing a single point of access to what is a single configuration is one okay use.

What I would do is go one step further and decouple the implementation of "get these global settings" from "by calling this static method." This is your option three.

Code example:

public class Settings {
  private static final Settings instance;
  static {
    // Insert code to create a Settings and populate instance dynamically here.
    // By default create a Settings, but allow tests to override with a subclass.
  }

  // Example of how to delegate:
  public static int getMaxWidgets() {
    return instance.getMaxWidgetsImpl();
  }

  protected int getMaxWidgetsImpl() {
    // Call the DB here, but allow subclasses to override.
    return ...;
  }
}

What this does is allow you to swap out (inject) a different implementation of the code that looks up settings using some mechanism. Maybe call a method to change it. In Java, I might use a system property with the class name. The point is, it is very easy to change the implementation and none of the application code is any wiser.