Compiler aided overloading

I was playing with xUnit.net for a pet project of mine. I wasn’t writing test cases with xUnit rather I was using the underlying xUnit engine to discover test cases and invoke calls that execute desired test cases. Forget the details of what I was doing, let us talk about it in a different post. But for now, I was consuming xUnit’s backend library.

XunitFrontController is the gateway to xUnit’s world; AFAIK. You create an instance of the controller specifying the target assembly.

var xfc = new XunitFrontController(
  AppDomainSupport.IfAvailable,
  "{full path of the assembly where the test cases reside}"
);

With the instance xfc created, you can now explore or find the test cases defined in the assembly, and even run desired or all test cases in the assembly. Cool, isn’t it?

Here is how to find test cases:

xfc.Find(
    true, // includeSourceInformation
    msi, // an instance of IMessageSink implementation
    TestFrameworkOptions.ForDiscovery()
);

The Find method does not return a list of test cases. Instead you provide an implementation of an IMessageSink implementation, which xUnit calls back on the OnMessage(IMessageSinkMessage) method. So xUnit calls your implementation of OnMessage with one of the three messages (in the case of Find):

  • TestCaseDiscoveryMessage (contains the list of TestCases discovered)
  • DiscoveryCompleteMessage
  • ErrorMessage

Alright, that was all about xUnit. Onto the real problem now …

Following is a typical implementation of IMessageSink:

/// <summary>
/// Not thread-safe but that's alright. Thread-safety not
/// relevant to our discussion.
/// </summary>
public class MyXunitWrapper : IMessageSink
{
    private readonly XunitFrontController xfc;
    private readonly TaskCompletionSource<IEnumerable<ITestCase>> onDiscoveryCompleteTask;
    private readonly List<ITestCase> testCases = new List<ITestCase>();

    #region Ctor(s)

    internal MyXunitWrapper(XunitFrontController xfc)
    {
        this.xfc = xfc;
        this.onDiscoveryCompleteTask = new TaskCompletionSource<IEnumerable<ITestCase>>();
    }

    #endregion

    #region Public Methods and Properties

    public async IEnumerable<ITestCase> GetAllTestCases()
    {
        xfc.Find(true, this, TestFrameworkOptions.ForDiscovery());
        return await this.onDiscoveryCompleteTask.Task;
    }

    #endregion

    #region IMessageSink Interface Implementation

    public bool OnMessage(IMessageSinkMessage message)
    {
        // Fancy C# 7.0's pattern-matching?
        if (message is TestCaseDiscoveryMessage)
        {
            OnMessage(message as TestCaseDiscoveryMessage);
        }
        else if (message is DiscoveryCompleteMessage)
        {
            OnMessage(message as DiscoveryCompleteMessage)
        }
        else if (message is ErrorMessage)
        {
            OnMessage(message as ErrorMessage)
        }
        else
        {
            // Log and swallow!
        }

        return true;
    }

    #endregion

    #region Private and Helper Methods

    private void OnMessage(TestCaseDiscoveryMessage message)
    {
        foreach (var testCase in message.TestCases)
        {
            Console.WriteLine($"Found nameof(XunitTestDiscoverer).test case: ${testCase.AsString()}");
            testCases.Add(testCase);
        }
    }

    private void OnMessage(DiscoveryCompleteMessage message)
    {
        onDiscoveryCompleteTask.SetResult(testCases);
    }

    private void OnMessage(ErrorMessage message)
    {
        // Log errors and ignore. Not much relevant to our discussion.
    }
    #endregion
}

I am sure you might have guessed already where I am getting at. Yes, the series of type checks in the OnMessage(IMessageSinkMessage message) method whose main purpose is routing to the overload for specialized handling of the message received. Before we go any further ….

The problem in discussion is not xUnit’s fault. It is by design probably with the goal of extensibility – add or remove message notifications in the future without affecting the IMessageSink interface. For instance, xUnit might want to add a new message notification DiscoveryStartedMessage. Such an approach is not uncommon.

A while back I worked on a project that had the same situation like a IMessageSink but a lot more methods in it.Déjà Vu.So imagine how long my OnMessage(IMessageSinkMessage message) would have been! Irrespective of the size, the pain is the same.

What I want is to get rid of the ifelse block that just does the routing. Instead leverage the specialized OnMessage overloads. Let us see if that is possible.

There is no way I will be able to get rid of OnMessage(IMessageSinkMessage message). If I do, the compiler is going to complain that I have not provided an implementation for IMessageSink interface. That means the best we can have is a smarter succinct routing code.

Remember dynamic?

public bool OnMessage(IMessageSinkMessage message)
{
    dynamic msg = message;
    OnMessage(msg);
}

public bool OnMessage(TestCaseDiscoveryMessage message) { ... }
public bool OnMessage(DiscoveryCompleteMessage message) { ... }
public bool OnMessage(ErrorMessage message) { ... }

That’s better, right? Don’t hold back, say it’s cool!

Except one problem. Or one of two problems.

  • If you happen to not provide a specialized OnMessage implementation, say for DiscoveryCompleteMessage, it would result in a stack overflow because OnMessage(msg) not able to find a match will end up calling itself.
  • Since the specialized methods have same names, it will result in a stack overflow. What if we have a different name, say HandleMessage. In that case, when a matching implementation of HandleMessage is not found, the DLR would throw a RuntimeBinderException.

It is just different flavors of the same problem. It is not a problem at all if you belong to the school of thought that don’t take, say null checks seriously, and defend saying why would you pass null if you know it is going to fail? 😂 You could convince yourself with rigorous code reviews making sure that no specialization is left out. Still, the integrity is approved neither by the compiler nor by the runtime.

What other ways can you think of to not write the boilerplate routing code that involves a series of type checks?

A short note on dynamic

Canonical examples of dynamic look something like this:

dynamic emp = new Employee();
emp.SetDesignation("Manager");

Such examples emphasize on the variable.Method() convention, where the Method call is evaluated at runtime by the DLR on the associated variable instance. A subtle thing to note in our case is the OnMessage(msg), which although not of the variable.Method() form, is also evaluated at runtime by the DLR because of the presence of a dynamic variable in the equation. You might already know this and spotted it in the code. I thought it was worth mentioning.

Dream

What would be really cool is a way to instruct the compiler to provide the routing implementation along with the safety of not running into the issues discussed above – stack overflow and RuntimeBinderException. The routing would not be based on dynamic but based on the specializations we provide. In other words, the compiler would do the ugly work for us by generating the type-check boilerplate. Consider the compiler-generated implementation as something similar to a default constructor, Equals or GetHashCode.

I suppose the instruction to the compiler has to be a keyword (not necessarily a new one). One way, just for kicks, is something like this:

public class MyXunitWrapper : dynamic IMessageSink
{
    // Specializations
    public bool OnMessage(TestCaseDiscoveryMessage message) { ... }
    public bool OnMessage(DiscoveryCompleteMessage message) { ... }
    public bool OnMessage(ErrorMessage message) { ... }
}
  • Note the interface IMessageSink above is prefixed with dynamic
  • Since the interface is marked dynamic, you don’t provide an implementation for OnMessage(IMessageSinkMessage). It is generated for you by the compiler with the routing.
  • If you provide an implementation of OnMessage(IMessageSinkMessage), it would be used as a fallback when other specializations do not match the message received. I believe the compiler has to use name mangling for all OnMessage overloads so that it does not mess with the compiler-generated implementation. The compiler-generated implementation would be the implementation of IMessageSink interface.
  • Specialized message handlers should have the same name (OnMessage) as the method on the interface.
  • Messages, for which specializations are not defined, are either ignored or routed to the OnMessage(IMessageSinkMessage) implementation if one is provided.

Alternatively, we can think of the compiler using dynamic as we had done earlier for the routing. That would be relatively slower due to the DLR overhead. Besides, error handling would be cumbersome for the compiler if not tricky; missing specializations and such.

I know it’s all a bit too much. I know there are many unanswered questions or unaddressed scenarios such as what if there is more than one method in IMessageSink. In any case, I enjoyed the exercise because I don’t like writing boilerplate, especially ones with (large) ifelses.

Although I spared my share of time to think about this problem, unfortunately, I don’t have a proposal of the sorts to aid implementing such a feature. That’s why it is a dream. 😜

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s