A More Functional Approach

In functional programming languages, you'll frequently see some variation of the car/cdr operations from LISP. Given a list, these functions can split it into the head (or first element) of the list, and the tail (the rest of the list). This is often used to traverse a list recursively: function f performs an operation on the head of the list, and then recursively calls f on the tail of the list.

I bring this up because we're talking about C# today. C# has plenty of fun functional programming features, and you can certainly use them to write very clean, comprehensible code. Or, like Nico's co-worker, you could do something like this.

private void HandleProcessPluginChain(ProcessMessage message, List<IProcessPlugin> processPlugins)
{
    if (processPlugins.Any())
    {
        processPlugins.First().Process(message, (msg) =>
        {
            HandleProcessPluginChain(msg, processPlugins.Skip(1).ToList());
        });
    }
}

public class EveryProcessPlugin : IProcessPlugin
{
	public void Process(ProcessMessage message, Action<ProcessMessage> onProcessCompleted)
        {
            ...

            onProcessCompleted(message);
        }
}

Let's start with the EveryProcessPlugin class. As you can see, it performs some process, and then calls an onProcessCompleted callback handler. That looks inoccuous, but is already pretty ugly. This code is synchronous, which means there's no reason to pass a callback- the way we get notified that the process has finished is that the method returns. Perhaps they wanted to expand this so that there could be async processes in the future, but that isn't what's happening here. Plus, C# has better constructs for handling async operations, and has had them for awhile.

But the real pain is in the HandleProcessPluginChain. Here, they've adapted the car/cdr approach to C#. If there are any elements in the list, we pop the first one off, and call its Process method. The callback we pass is a recursive reference to HandleProcessPluginChain where we Skip the first element (a C# cdr) to pass the tail of the list to the recusive call.

Key language features that make this approach efficient in functional languages don't exist here. C# doesn't support tail-call optimizations, so even if the compiler could see that this was tail calls (I'm not certain about that, with the lambda in there), C# wouldn't benefit from that anyway. The fact that we need to pass a List and not an IEnumerable means that every call to ToList is visiting every member in the list to construct a new List object every time.

Maybe this is a case where someone was coming from F#, or Haskell, or wished they were using those languages. The fact that it's not conventional C# isn't itself terrible, but the fact that it's trying so hard to be another language is.