Magic numbers - Reading from a config file the same as global space? Bad for unit testing?

Question

Consider the following class:

class Something : ISomething {

    public void DoesSomething(int x) {
        if (x == 0) {
            x = 1;
        }
    }
}

I want to of course remove the magic number - my unit tests are passing etc... but I want to refactor the horrible magic number out.

I'm using C# but I guess this problem is pretty generic. Reading from a config file (xml file) is done by something like:

ConfigurationManager.AppSettings["MyOldMagicNumber"]...

Which of course, would be a sod to test. I could easily make a private function in this class that is marked virtual. Its purpose to encapsulate that above code. This would allow me to access in my unit tests to override and plug in my own value.

My question is -

Is this bad what I'm doing? See title.

Edit:

This is for a game - therefore is more than likely during development the values will change often and doing a re-build will be a chore. I should have mentioned that, the above code is generic, I made it so to keep the question as simple as possible. A bit of context though - that '0' is the game area bounds.

Thanks in advance.

Answer 1

Do you really need to change these values without recompiling the program? If no, I think you should avoid putting it in your config file, otherwise you will end up programming in xml ;)

Using constants is good for readability in this case:

class Something : ISomething {

    public const int Zero = 0;
    public const int One = 1;

    public void DoesSomething(int x) {
        if (x == Zero) {
            x = One;
        }
    }
}

Answer 2

Why don't you create an interface for this like

public interface IApplicationSettings {
 int MyOldMagicNumber { get; }
}

Then have two implementations of this, one for production that reads from the config file and one fake for unit-tests.

public class ApplicationSettings : IApplicationSettings {
 public int MyOldMagicNumber { 
   get { return ConfigurationManager.AppSettings["MyOldMagicNumber"]; }
  }
}

public class FakeApplicationSettings : IApplicationSettings {
 public int MyOldMagicNumber { 
   get { return 87; /*Or whatever you want :) */ }
  }
}

Answer 3

I would definitely put it in a config file if you are doing a game. This is because you may (will) want to try out a different set of values. Putting it in a config file means you can swap out files and try new numbers without modifying the "engine". You may even find yourself defining the properties/values of every game critter in the config and this is ok!