1

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.

7
  • Yes. Otherwise, how can you actually test your code? You might want to take the time to watch this. The database is a detail.
    – RubberDuck
    Mar 30 '15 at 20:20
  • If no, then it is option 1. Which I can do. Separating out the DB completely is not possible within any time frames we'd ever have. Apr 1 '15 at 14:48
  • You're missing the point of unit testing here. No dependencies on external resources. Option 1 is not an option.
    – RubberDuck
    Apr 1 '15 at 14:52
  • @RubberDuck In some ways you are absolutely correct, I do always miss the point of unit testing, it seems to take a lot of time and not really catch anything that integration tests miss. I can't not have integration tests, so unit testing is a huge and pointless waste of effort in all my experiences. But that aside, if I never had plans to unit test, would the refactor still be worth it? Apr 19 '15 at 9:58
  • Absolutely worth it. What happens if (when) someone decides that XML would be a better way to store the config? The way you're doing things now, you're going to have to change your code all over the place. Using an interface and injecting the dependency makes your coupling looser. Loose coupling is always a good thing. You should be able to swap out the source of that data without changing a single line of code. You should be able to create a brand new class that supplies that data and swap it out for your db.
    – RubberDuck
    Apr 20 '15 at 11:20
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.
1
  • I like "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." that is a very valid point, the problem is far down in some libraries they too use the static settings, so until the dependencies are also updated I still need to mock until they too are updated. Though it is indeed better to not make things worse by propagating the old "wrongs" into new code. Apr 19 '15 at 10:10
1

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.

2
  • You are right in that a singleton could be injected into Option 3, but most uses of singletons are that they aren't injected e.g. rather than have StaticClass.MySetting in the code I would write SingletonClass.Instance.MySetting, which is an Option 4 :) Apr 19 '15 at 10:03
  • Though when using the Singleton, you would mock the static Instance property, which is a bit of option 2 also. Apr 19 '15 at 10:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.