C#: How to shoot yourself in the foot with a generic extension method

TL;DR: wrapping existing method DoSomething(MyData data), with an identically named generic extension method DoSomething<T>(T data) is a bad idea, since it erases compile-time type checking. DoSomething() may now be called with any type, and it may be hard to tell whether you are calling the original or the wrapper.

The following is a true story, even though some details have been changed to protect the innocent. Suppose we have a messaging library along these lines:

interface IMessage
{
    Type Type { get; }
    object Data { get; }
}

class Message : IMessage
{
    public Type Type { getset; }
    public object Data { getset; }
}

class Sender
{
    public void Send(Message message)
    {
        // ...do actual sending here...
        Console.WriteLine($"Send: Type={message.Type.Name}, Data={message.Data}");
    }
}

Expressions like sender.Send(new Message {Type = typeof(int), Data = 42}) quickly become an annoyance, so we create an extension method to simplify them:

static class SenderExtensions
{
    public static void Send<T>(this Sender sender, T data)
    {
        sender.Send(new Message {Type = typeof(T), Data = data});
    }
}

Now we can simply write sender.Send(42)and it works as expected. But… Consider this code:

var message = new Message {Type = typeof(int), Data = 42};
sender.Send(message); // prints Send: Type=Int32, Data=42

var iMessage = (IMessage) message;
sender.Send(iMessage); // oops! Type=IMessage, Data=ShootFoot1.Message

We cast our message to IMessage type, but the Send() method expects parameter of type Message. This by itself is probably a design error, but let’s ignore it. ¡No problemo! says the compiler. Passing IMessage compiles, but it calls the extension method instead of the regular one, and wraps our message in another Message object.

The same would have happened if we had a generic overload of Send(), but extension method is especially evil, since it is invisilbe. You cannot guess its existence by looking at either the call site or the definition of the Sender class. I personally marveled at the wrapped message in debugger for at least 10 minutes before I realized what’s going on. Obviously, the real program was much more complex and had many additional details that caused distraction.

Moral of the story: if you create a generic extension method like this, give it a distinct name. E.g. if the extension method were called SendObject(), then Send(iMessage)would simply have not compiled.

Complete code:
The original, buggy version
Fixed version with renamed extension method

Leave a Reply

Your email address will not be published. Required fields are marked *