Overloading the meaning of configuration settings is a smell

Is there anything wrong with this?

if (settings[“mysetting”] != string.Empty) 
{ 
    DoSomethingWith(settings[“mysetting”]);
}

The meaning of the configuration setting "mysetting” is overloaded. One of its possible values is special, and is used to turn some behaviour on, or off. It’s like overloading the return value of a counting method to handle error codes as well as real values; like returning –1 on failure to count, and the real count if successful.

Notionally, although the empty string is in the valid set of possible strings, it may not be in the possible set of valid values for this particular setting, in which case you might argue that it’s reasonable to take one of the invalid values and overload on it. However, that hides the fact that there’s a logical difference between turning a behaviour on, or off, and configuring that behaviour. Generally it leads to branching in the code, which makes it more complex.

That’s apart from the fact that the empty string might actually be a valid value one day.

Instead, it makes sense to think about why you need to turn the behaviour on and off, rather than just configuring it. We hit this problem recently when we wanted to add some configuration that allowed us to replace a specific string with another string[*]. Specifically we wanted to override the name of a callback numerical solver in a task definition. Originally it was using “NewtonRaphson”, and we wanted to use “SomeNAGMethod”.

I mulled over two approaches with a friend – first, hardcoding the new value and having a bool configurable flag to use it or not; and a single string configurable value for which string.Empty turns the behaviour off, and all other values actually configure the behaviour.

<add key=“FixSolverMethod” value=“false”/>
...
settings[“FixSolverMethod”] = “true”;  // to override

or

<add key=“SolverMethod” value=“”/> 
... 
settings[“SolverMethod”] = “SomeNAGMethod”; // to override

Both were tied to the idea that this was behaviour that needed to be turned off an on, and as a result implied the need for decision making at the point of use, as well as actual use of the value.

if(setting[“FixSolverMethod”]) FixSolverMethod(); 
    // which then uses a hardcoded method name

or

if(setting[“SolverMethod”] != string.Empty) 
    OverrideSolverMethod(setting[“SolverMethod”]);

I even toyed with the idea of configurable configuration items:

<add key=”SolverMethod” value=”SomeNAGMethod” enabled=”false”/>

but that way madness lay.

It felt wrong to me, and I couldn’t figure out why until I spotted this smell; that the meaning of the value was being overloaded. I realised then that I could simply extend the notion of what we were doing. Instead of turning the behaviour on or off, I could just turn leave it on and have the default value for the configurable setting set the same as the existing value.

// at initialization – gives us a default
<add key=(“SolverMethod”, value=“NewtonRaphson”/>  

...
// some time later, when we want to reconfigure. alternatively we could
// just set the new method name in the xml above ...
settings[“SolverMethod”] = “SomeNAGMethod”;  

...
// finally, just use it
UpdateSolverMethod(setting[“SolverMethod”]); 

In this case no branching is needed – the solver method name is always overwritten. At worst it’s a wasted operation, as it’s overwritten with the string that’s already there.

Once you get to that point you start thinking about the code more generally. Is this the right place to be doing this? If we’re having to fix this up in every case, can’t we do it where these definitions are created? That led us to explore parts of the codebase that we didn’t have direct experience of, and what did we find? That we had control over the tasks definitions directly – we were creating them, but in a part of the code neither of us knew about. We could keep the configuration setting we’d created, but apply it at source, logically and consistently, to configure the tasks instead of patching them up downstream.

Overloading configuration settings is a code smell. It’s worth thinking – can I set a default that matches the existing behaviour, and apply it all the time? And if I can, does that mean it’s better done somewhere else?

[*] Ah, the glamour of corporate development!

Out (and ref) parameters in Actions, Funcs, delegates and lambdas

I ran into a couple of wrinkles I wasn’t aware of before when coding up a facade for the ThreadPool for tests today, along the lines of the Path, Directory etc facades I’ve done before.

I wanted to insert a replacement ThreadPool so I could test the GetAvailableThreads method, which takes two out parameters.

You can’t quite specify the lambda as you might expect for the method implementation. This initial attempt, for example, doesn’t work:

public static class ThreadPool
{
    public static Action<int, int> GetAvailableThreadsImpl =
        System.Threading.ThreadPool.GetAvailableThreads;

    public static void GetAvailableThreads(out int workerThreads, out int ioThreads)
    {
        GetAvailableThreadsImpl(out workerThreads, out ioThreads);
    }
}

I suspect this is to do with the calling style here -- but need to understand why it happens in more detail. You can work around it by just creating your own delegate class:

public delegate void GetAvailableThreadsDel(out int x, out int y);
 
public static class ThreadPool
{
    public static GetAvailableThreadsDel GetAvailableThreadsImpl =
        System.Threading.ThreadPool.GetAvailableThreads;

    public static void GetAvailableThreads(out int workerThreads, out int ioThreads)
    {
        GetAvailableThreadsImpl(out workerThreads, out ioThreads);
    }
}

Then follows a small wrinkle when defining a replacement behaviour in your test. You can, of course, create your own method with the right signature:

[Test]
public void Blah()
{
    int callCount = 0;
    ThreadPool.GetAvailableThreadsImpl = GetAvailableThreadsStub;
}

private int callCount = 0;
private void GetAvailableThreadsStub(out int x, out int y)
{
    if (callCount == 0)
    {
        x = 5;
        y = 5;
    }
    else
    {
        x = 10;
        y = 10;
    }

    callCount++;
}

It turns out you can define out parameters in a lambda however -- but you must also explicitly state the parameter type as well, unlike normal:

[Test]
public void Blah()
{
    int callCount = 0;
    ThreadPool.GetAvailableThreadsImpl = (out int x, out int y) =>
        {
            if (callCount == 0)
            {
                x = 5;
                y = 5;
            }
            else
            {
                x = 10;
                y = 10;
            }

            callCount++;
        };
    }
}

Again, I'm not entirely sure how it works here, and need to investigate further. For now, just a wrinkle to remember.