Geeks With Blogs
Simon Cooper Peering into the depths of .NET

It is an oft-repeated maxim that you shouldn't add methods to a publically-released interface in an API. Recently, I was hit hard when this wasn't followed.

As part of the work on ApplicationMetrics, I've been implementing auto-reporting of MVC action methods; whenever an action was called on a controller, ApplicationMetrics would automatically report it without the developer needing to add manual ReportEvent calls. Fortunately, MVC provides easy hook when a controller is created, letting me log when it happens - the IControllerFactory interface.

Now, the dll we provide to instrument an MVC webapp has to be compiled against .NET 3.5 and MVC 1, as the lowest common denominator. This MVC 1 dll will still work when used in an MVC 2, 3 or 4 webapp because all MVC 2+ webapps have a binding redirect redirecting all references to previous versions of System.Web.Mvc to the correct version, and type forwards taking care of any moved types in the new assemblies. Or at least, it should.

IControllerFactory

In MVC 1 and 2, IControllerFactory was defined as follows:

public interface IControllerFactory
{
    IController CreateController(RequestContext requestContext, string controllerName);
    
    void ReleaseController(IController controller);
}
So, to implement the logging controller factory, we simply wrap the existing controller factory:
internal sealed class LoggingControllerFactory : IControllerFactory
{
    private readonly IControllerFactory m_CurrentController;
    
    public LoggingControllerFactory(IControllerFactory currentController)
    {
        m_CurrentController = currentController;
    }
    
    public IController CreateController(
        RequestContext requestContext, string controllerName)
    {
        // log the controller being used
        FeatureSessionData.ReportEvent("Controller used:", controllerName);
        
        return m_CurrentController.CreateController(requestContext, controllerName);
    }
    
    public void ReleaseController(IController controller)
    {
        m_CurrentController.ReleaseController(controller);
    }
}

Easy. This works as expected in MVC 1 and 2. However, in MVC 3 this type was throwing a TypeLoadException, saying a method wasn't implemented. It turns out that, in MVC 3, the definition of IControllerFactory was changed to this:

public interface IControllerFactory
{
    IController CreateController(RequestContext requestContext, string controllerName);
    
    SessionStateBehavior GetControllerSessionBehavior(
        RequestContext requestContext, string controllerName);
        
    void ReleaseController(IController controller);
}

There's a new method in the interface. So when our MVC 1 dll was redirected to reference System.Web.Mvc v3, LoggingControllerFactory tried to implement version 3 of IControllerFactory, was missing the GetControllerSessionBehaviour method, and so couldn't be loaded by the CLR.

Implementing the new method

Fortunately, there was a workaround. Because interface methods are normally implemented implicitly in the CLR, if we simply declare a virtual method matching the signature of the new method in MVC 3, then it will be ignored in MVC 1 and 2 and implement the extra method in MVC 3:

internal sealed class LoggingControllerFactory : IControllerFactory
{
    ...
    
    public virtual SessionStateBehaviour GetControllerSessionBehaviour(
        RequestContext requestContext, string controllerName) {}
    
    ...
}

However, this also has problems - the SessionStateBehaviour type only exists in .NET 4, and we're limited to .NET 3.5 by support for MVC 1 and 2.

This means that the only solutions to support all MVC versions are:

  1. Construct the LoggingControllerFactory type at runtime using reflection
  2. Produce entirely separate dlls for MVC 1&2 and MVC 3.

Ugh. And all because of that blasted extra method!

Another solution?

Fortunately, in this case, there is a third option - System.Web.Mvc also provides a DefaultControllerFactory type that can provide the implementation of GetControllerSessionBehaviour for us in MVC 3, while still allowing us to override CreateController and ReleaseController.

However, this does mean that LoggingControllerFactory won't be able to wrap any calls to GetControllerSessionBehaviour. This is an acceptable bug, given the other options, as very few developers will be overriding GetControllerSessionBehaviour in their own custom controller factory.

So, if you're providing an interface as part of an API, then please please please don't add methods to it. Especially if you don't provide a 'default' implementing type. Any code compiled against the previous version that can't be updated will have some very tough decisions to make to support both versions.

Cross-posted from Simple Talk. Posted on Thursday, March 8, 2012 3:27 PM | Back to top


Comments on this post: Why you shouldn't add methods to interfaces in APIs

# re: Why you shouldn't add methods to interfaces in APIs
Requesting Gravatar...
So how do you propose they evolve the interface? Add a new one to each version?
Left by Ben on Mar 08, 2012 7:37 PM

# re: Why you shouldn't add methods to interfaces in APIs
Requesting Gravatar...
Ideally, you wouldn't. I accept this isn't a realistic answer. Either you should create a new interface that implements the old one (hopefully named something more descriptive than IControllerFactory2), or, at the very least, provide a base class implementing the interface with overridable methods, similar to DefaultControllerFactory. It is a tricky problem, with no clear & easy solution.
Left by Simon Cooper on Mar 09, 2012 8:38 AM

Your comment:
 (will show your gravatar)


Copyright © simonc | Powered by: GeeksWithBlogs.net