Introducing Interfaces into Existing Code

March 19, 2009

I was in a situation with the project I’ve recently joined that I needed a subclass to have different behaviour from one of its dependencies. The dependency was directly coupled to the class, not only through being directly instantiated by the class (not what I am trying to solve here, but definitely on my hit-list) but also being held as a reference to that particular type. Here’s a (very simplified) example:

class SomeServiceClass
{
    public void ProvideService()
    {
        Console.WriteLine("Default Service behaviour");
    }
}

class SomeBaseClass
{
    public SomeBaseClass()
    {
        m_service = new SomeServiceClass();
    }

    protected SomeServiceClass m_service;

    public void DoSomething()
    {
        m_service.ProvideService();
    }
}

class SomeDerivedClass : SomeBaseClass
{
}

NOTE: Exposing the m_service field to sub-classes is not something I consider to be good practice. There are obviously further problems with this code that I am NOT addressing here.

I needed SomeServiceClass to behave differently when invoked from SomeDerivedClass. The service was sufficiently complex (read: violated the Single Responsibility Principle big-time) that subclassing it didn’t particularly appeal. Introducing an Interface would allow me to alter the service entirely for SomeDerivedClass whilst leaving SomeBaseClass and all its other sub-classes (oh yeah, did I mention it was only one of many sub-classes that needed this altered behaviour?) with the existing code.

First off, define the interface. As this is being derived from an existing class and the idea is to make as little change to existing code as possible, the interface should end up looking exactly the same as the public signature of the class:

interface ISomeService
{
    void ProvideService();
}

This allows SomeBaseClass to redefine the m_service field as type ISomeService and the DoSomething() method is none the wiser. However, the constructor explicitly creates a SomeServiceClass instance to assign to it. Since we made the interface the same, we could just make SomeServiceClass implement ISomeService, but it would be nice if we could leave that class unchanged. Instead, it would be great to present an actual SomeServiceClass instance looking like an ISomeService. This can be done with the Adapter Pattern. We can make a new class that implements ISomeService and delegates all calls to an underlying SomeServiceClass instance. It would look something like this:

class SomeServiceAdapter : ISomeService
{
    public SomeServiceAdapter(SomeServiceClass baseServiceImplementation)
    {
        m_baseServiceImplementation = baseServiceImplementation;
    }

    private SomeServiceClass m_baseServiceImplementation;

    #region ISomeService Members

    public void ProvideService()
    {
        m_baseServiceImplementation.ProvideService();
    }

    #endregion
}

This allows us to declare and initialise the m_service field as follows:

class SomeBaseClass
{
    public SomeBaseClass()
    {
        m_service =
            new SomeServiceAdapter(new SomeServiceClass());
    }

    protected ISomeService m_service;

    public void DoSomething()
    {
        m_service.ProvideService();
    }
}

Now that our consuming class is referencing an interface, we can supply a different implementation for the derived class. As I mentioned above, I would much rather see the implementation passed in from outside this class, but not every architectural weakness can fixed at once, so we’ll work with what we’ve got for now, and we have got is a protected field that the derived class can access. So, instead of using the instance that the base class initialises it to, the derived class can override it in its constructor. But first, of course, we need to have an alternate implementation to override it with:

class SomeServiceAlternative : ISomeService
{
    #region ISomeService Members

    public void ProvideService()
    {
        Console.WriteLine("Alternative Service behaviour");
    }

    #endregion
}

Our derived class can then do this:

class SomeDerivedClass : SomeBaseClass
{
    public SomeDerivedClass()
    {
        m_service = new SomeServiceAlternative();
    }
}

Again, I’d like to stress that Dependency Injection would be the way to go if you have the choice. In this instance, I had a specific sub-class that wanted different behaviour, and as such I was able to get the sub-class to define its preference. We are now additionally coupled to the not just the original service class (still being new-ed up in the base class constructor) but also the new interface and its two implementations. However, I took the view that the dependencies on the classes being confined to just the constructor was a better state than dependening on the service class throughout the base class. Injecting these classes in from the outside would reduce the dependencies to just the interface.


Wrapping HttpContextBase for Dependency Injection

January 28, 2009

I wanted to remove my controllers’ dependencies on HttpContext. I don’t know about anyone else, but having to set a fake context in all my unit tests was driving me nuts. Yeah, sure, Stephen Walther’s Fake Contexts are great, but they’re just another piece of friction in the unit test. What I currently need to access the HttpContext for is to get the authenticated user, as I was talking about in my last post. I ended up with my SessionController looking like this:

public class SessionController : Controller
{
	public ViewResult Authentication()
	{
		Authentication auth = 
		  new Authentication(ControllerContext.HttpContext.User);
		return View(auth);
	}
}

Okay, time to split that dependency out of there. First off, I create a simple User class to represent a user and altered the Authentication class to take an instance of that rather than IPrincipal to further divorce my app logic from the underlying framework. I can now define an interface to represent the session:

public interface ISession
{
	User AuthenticatedUser();
}

This is currently all I need to know about the session, so I’ve followed the YAGNI principle and left the interface at that for now. I’ve got two implementations for this interface, first up is FakeSession for testing purposes:

class FakeSession : ISession
{
	private User mAuthenticatedUser;
	public void Unauthorise()
	{ mAuthenticatedUser = null; }
	public void AuthoriseAs(User authedUser)
	{ mAuthenticatedUser = authedUser; }

	#region ISession Members

	public JkEditingHub.Models.User AuthenticatedUser()
	{
		return mAuthenticatedUser;
	}

	#endregion
}

This is pretty simple and allows me to set who is logged in with ease.

The other implementation simply wraps the HttpContextBase that the controller will know about:

public class AspSession : ISession
{
	public AspSession(HttpContextBase context)
	{
		mContext = context;
	}

	private HttpContextBase mContext;

	public User AuthenticatedUser()
	{
		if (!mContext.User.Identity.IsAuthenticated)
		{ return null; }

		return new User(mContext.User.Identity.Name);
	}
}

Great, now all I need to do is inject the ISession into any controllers that need it. I’m using constructor injection, so SessionController now looks like this:

public class SessionController : Controller
{
	public SessionController(ISession session)
	{
		mSession = session;
	}

	private ISession mSession;

	public ViewResult Authentication()
	{
		Authentication auth =
		  new Authentication(mSession.AuthenticatedUser());
		return View(auth);
	}
}

Okay, now all I have to do is get StructureMap to use AspSession as its default for ISession. In my bootstrapping code I can do this:

ObjectFactory.Initialize(x =>
{
	// Other registrations...
	x.ForRequestedType<ISession>().TheDefaultIsConcreteType<AspSession>();
});

Great, no problem here. But how can I tell StructureMap which object to use for the HttpContextBase argument in the AspSession constructor, when that is going to be a specific instance that is different each time and not available when I’m bootstrapping? Fortunately, this is very very easy and doesn’t require going much beyond the very basic use I’ve so far made of StructureMap. In my ControllerFactory I am using StructureMap to create the instances of controllers:

protected override IController GetControllerInstance(Type controllerType)
{
	return (IController)ObjectFactory.GetInstance(controllerType);
}

The ControllerFactory has a handle to the RequestContext which contains the HttpContext for the request. All we need to do is tell StructureMap that if it requires an HttpContextBase, then use this object by using the With<T>(T arg) method. As I said, not at all advanced usage by any measure:

protected override IController GetControllerInstance(Type controllerType)
{
	return (IController)ObjectFactory
		.With<HttpContextBase>(this.RequestContext.HttpContext)
		.GetInstance(controllerType);
}

Sub-Controllers, PartialRequests and Separating Views From Controllers

January 26, 2009

Earlier, I blogged about an alternative to sub-controllers. That post wasn’t about sub-controllers being rubbish, rather that the problem I was trying to solve wasn’t the same as the problem that sub-controllers were meant to solve.

Recently, I’ve been looking at another problem in my code: all the stuff that has to appear on every page. Currently this is confined mostly to the login box that appears on the top right. When you’re not authenticated, it shows as a login form. When you are authenticated, it displays who you’re logged in as and gives you a logout link.

As this appears on every page, it’s defined in the master page and every controller has to return a model with the appropriate data in it. My post on DRYing out my unit tests showed one aspect of my solution to this. I defined a MasterPageData class that held all the models required for the master page to work, and derived from this a PageData<TModel> that also supplied a model that the page worked on. All my views were then able to access the (strongly-typed) model and the master page could access all the information (also strongly-typed) it needed too.

That just left how to populate the data. To do this, I defined two overloads of a new method JkhubView() in my base controller to be used in place of View(), one that took no arguments and returned a ViewResult<MasterPageData>, the other was a generic method that took a TModel and returned a ViewResult<PageData<TModel>>. Both of these methods populated the data in the MasterPageData object with the appropriate information.

This works. Every controller action calls one of the overloads of the JkhubView method and the strongly typed master page gets the data it needs. However, it’s not great for the following reason:

Every controller action makes an assumption about the view that will render its result.

That is, it assumes that the view will require this data about whether the user is logged in or not. This is completely irrelevant to a controller action that, for example, generates a list of a user’s projects. I’m keen to get the separation of controllers and views right. What’s the point in using an MVC framework if you’re going to mash everything up together anyway? I’ve pondered the question of what belongs in the controller and what belongs in the view before.

When I had decided that this just wasn’t the right thing for the controller to be concerned about, I decided to have an investigate of sub-controllers. I got the MVC Contrib project hooked up to my application, re-read Matt Hinze’s blog post on the subject and started creating my sub controller and view for the session status. Great, this is gonna be cool. Right then, let’s wire this up to my pages: all I need to do is… wait, I pass the sub-controller to my controller action to place it in the ViewData?

Okay, first off, I don’t like using the ViewData. I much prefer a strongly typed model to give me everything I need (that’s why I’ve got the MasterPageData class floating about). So that’s a bit sucky.

But more than that. The controller action is still making that same assumption about what the view is going to need. Sucky.

Okay, not to worry, I recall reading about an alternative approach to this problem. Steve Sanderson posted a neat little trick to fire off another request to the MVC pipeline. The only problem with his implementation is that his controllers are creating the PartialRequest and stuffing it in the ViewData again. Those were my two sticking points with sub-controllers! However, a quick modification to the HtmlHelper extension he suggests allows the View to create the PartialRequest inline:

public static void RenderPartialRequest(this HtmlHelper helper, string controller, string action)
{
	PartialRequest request = new PartialRequest(new { controller, action });
	request.Invoke(helper.ViewContext);
}

Instead of passing the ViewData’s key, we tell the extension method which Controller and Action to create the request for. I’ve not yet found any downsides to this over creating the PartialRequest in the controller. I may find some issues when I come to look at the partial output caching that Steve blogs about in his next post, so as with all things I’m building here, this advice is subject to change without warning.


Writing HtmlHelper extension methods

January 19, 2009

I’ve created some pretty crazy routes for my ASP.NET MVC application, requiring some overly verbose calls to the RouteLink method in my views:

<ul>
<% foreach (var p in Model.ProjectList)
{ %>
	<li><%= Html.RouteLink(
		p.ProjectName,
		"ProjectSubRoutes",
		new RouteValueDictionary(
			new { controller = "Projects",
				editorRef = p.ProjectOwner.EditorName,
				projectRef = p.ProjectRef }),
		new RouteValueDictionary())%></li>
<% } %>
</ul>

That’s what I need to do to get a link to a project’s home page. Which, given the site is all about hosting these projects, I’m going to want to do quite a lot.

The answer is to provide my own helper methods for the views. The RouteLink method is defined as an extension method to the HtmlHelper class, which means that it is easy to make your own methods to augment the set supplied by the framework. The advantage is that you can make your methods domain-aware. If I want to have a link to a project page, I can simply create an extension method that takes an instance of a project:

public static class HtmlHelperExtensions
{
	public static string GenerateProjectLink(
		this HtmlHelper helper,
		ProjectPresenter project)
	{
		return helper.RouteLink(
			project.ProjectName,
			"ProjectSubRoutes",
			new RouteValueDictionary(
				new { controller = "Projects",
					editorRef = project.ProjectOwner.EditorName,
					projectRef = project.ProjectRef }),
			new RouteValueDictionary());
	}
}

I don’t need to worry about constructing the HTML for the link by hand, I can still use the supplied helper methods under the hood, but I save my views from being polluted with wiring details, as well as being more DRY. The view now looks like this:

<ul>
<% foreach (var p in Model.ProjectList)
{ %>
	<li><%= Html.GenerateProjectLink(p) %></li>
<% } %>
</ul>

This is more readable. Before you would have had to ferret around a bit to work out what the link was for. Now the method name clearly tells you that it is generating a project link. You don’t need to mentally duplicate the work of the routing mechanism to figure out where that request is going to end up.


Commentless Code is not a Silver Bullet

January 19, 2009

He doesn’t say it is a silver bullet, but Jeffrey Palermo has blogged that limiting code comments increases maintainability. This is definitely something I’ve tried to impress on my current team members. I would only like to add one point, and that is that the approach doesn’t automatically mean your variable names stay relevant.

Maintenance developers will still be able to change the semantic meaning of the variables. Do you really think they’re going to bother changing the name of the variable?


DRYing Out NUnit Tests

January 13, 2009

In my ASP.NET MVC application, I have created a new method in my base controller that will return a ViewResult populated with standard “every-page” data, such as whether a user is logged in and what should appear in the dynamic portions of the MasterPage, and so on. This method has two versions, a no-arguments method for when the page itself needs no data and a generic version that can take an object representing any data for the page, and will look something like this:

public class JkhubController : Controller
{
	// ...

	public ViewResult JkhubView()
	{
		JkhubMasterPage resultModel = new JkhubMasterPage();
		
		// populate the common data here
		
		return View(resultModel);
	}
	public ViewResult JkhubView(TModel model) where TModel : class
	{
		JkhubPage resultModel = new JkhubPage(model);
		
		// populate the common data here
		
		return View(resultModel);
	}
}

I wanted to unit test both methods returned the correct common data for a range of scenarios, without having to duplicate the tests. I discovered it was possible to do by placing the tests in an abstract class and having two derived classes that implemented the method call that the tests exercised. Like this:

public abstract class JkhubControllerJkhubViewCommonTests
{
	private JkhubController jkhubController;
	protected JkhubController Controller { get { return jkhubController; } }
	
	[SetUp]
	public void SetUp()
	{
		jkhubController = new JkhubController();
		SetControllerContext(new FakeControllerContext(jkhubController));
	}
	
	private void SetControllerContext(ControllerContext context)
	{
		jkhubController.ControllerContext = context;
	}
	
	protected abstract JkhubMasterPage JkhubViewResultModel();
	
	[Test]
	public void JkhubViewReturnsViewResultWithCorrectModelType()
	{
		ViewResult viewResult = jkhubController.JkhubView();
		
		Assert.That(viewResult.ViewData.Model, Is.InstanceOfType(typeof(JkhubMasterPage)));
	}
	
	[Test]
	public void AuthenticatedFlagIsFalseWhenUserIsNotAuthenticated()
	{
		JkhubMasterPage resultModel = JkhubViewResultModel();
		
		Assert.That(resultModel.Auth.Authenticated, Is.False, "User should not be authenticated");
	}

	[Test]
	public void AuthenticatedFlagIsTrueAndUsernameIsSetWhenUserIsAuthenticated()
	{
		string userName = "User";
		SetControllerContext(new FakeControllerContext(jkhubController, userName));
		JkhubMasterPage resultModel = JkhubViewResultModel();
		
		Assert.That(resultModel.Auth.Authenticated, Is.True, "User should be authenticated");
		Assert.That(resultModel.Auth.Username, Is.EqualTo(userName), "Username should be set to the name of the authenticated user");
	}
}

[TestFixture]
public class JkhubControllerJkhubViewTests : JkhubControllerJkhubViewCommonTests
{
	protected override JkhubMasterPage JkhubViewResultModel()
	{
		return (JkhubMasterPage)(Controller.JkhubView().ViewData.Model);
	}
}

[TestFixture]
public class JkhubControllerJkhubViewTModelTests : JkhubControllerJkhubViewCommonTests
{
	protected override JkhubMasterPage JkhubViewResultModel()
	{
		String model = "Model";
		return (JkhubMasterPage)(Controller.JkhubView(model).ViewData.Model);
	}
}

Note that the abstract class isn’t marked with the TestFixture attribute. If you load that up in the NUnit GUI, it reports the names of the tests prefixed with the class that they are defined in:

dryingoutnunittests

This will mean that any further tests I write the common functionality will automatically get run for both methods, without having to wire up a test in two separate fixtures.


Hierarchical Controllers in ASP.NET MVC

January 9, 2009

When I read that Jeffrey Palermo had added sub-controllers to ASP.NET MVC, I hoped that it would be the exact solution I was looking for in my app. Unfortunately, the sub-controllers in this context meant something different to what I understood when I first read the term.

What I wanted is perhaps better named “Hierarchical Controllers”. The common pattern with ASP.NET MVC is to have a controller for each of your entities. However, my domain has some entities that “own” other entities in such a manner that accessing those owned entities at the top level didn’t make sense.

Here’s the concrete example. I have an entity that represents a project. Anyone on that project may post a project news item. Instead of having urls like this:

http://mywebapp/Projects/ProjectX
http://mywebapp/News/ProjectX_NewsItemY

I wanted to route them like this:

http://mywebapp/Projects/ProjectX
http://mywebapp/Projects/ProjectX/News/NewsItemY

Not a problem, you might think, just define a new route for the news item display

Projects/{projectId}/{controller}/{newsId}

I initially started out with that, but it became apparent that it fit better with ASP.NET MVC’s convention-over-configuration paradigm to be able to just create a new controller with the name like {toplevelentity}_{subentity}Controller and have things wired up automatically for me. Here’s what I came up with:

First off, define my route:

routes.MapRoute(
	"RouteWithSubController",
	"{controller}/{topId}/{subController}/{action}/{subId}",
	new { topId = "", subController = "Home", action = "Index", subId = "" }
);

If subController is defined, we want to return a different type of controller than ASP.NET MVC will get by default, so we need a custom ControllerFactory:

public class MyControllerFactory : DefaultControllerFactory
{
	protected override Type GetControllerType(string controllerName)
	{
		object subControllerName;
		if (RequestContext != null
			&& RequestContext.RouteData.Values.TryGetValue("subController", out subControllerName))
		{
			return base.GetControllerType(String.Format("{0}_{1}", controllerName, (string)subControllerName));
		}
		
		return base.GetControllerType(controllerName);
	}
}

Don’t forget to register this in Global.asax:

protected void Application_Start()
{
	RegisterRoutes(RouteTable.Routes);
	ControllerBuilder.Current.SetControllerFactory(new MyControllerFactory());
}

This will call the correct controller if you follow the new convention. However, it messes up when identifying the view.

I opted for the convention here of placing the views in the same folder as those for the top-level controller and naming them {subController}_{action} rather than just {action}. To get this working, we need to subclass the default WebFormViewEngine:

public class MyWebFormViewEngine : WebFormViewEngine
{
	public override ViewEngineResult FindView(ControllerContext controllerContext, string viewName, string masterName)
	{
		object subControllerName;
		if (controllerContext != null
			&& controllerContext.RouteData.Values.TryGetValue("subController", out subControllerName))
		{
			return base.FindView(
				controllerContext,
				String.Format("{0}_{1}", (string)subControllerName, viewName),
				masterName);
		}
		return base.FindView(controllerContext, viewName, masterName);
	}
}

And, again, register it in Global.asax:

protected void Application_Start()
{
	RegisterRoutes(RouteTable.Routes);
	ControllerBuilder.Current.SetControllerFactory(new MyControllerFactory());
	ViewEngines.Engines.Clear();
	ViewEngines.Engines.Add(new MyWebFormViewEngine());
}

Follow

Get every new post delivered to your Inbox.