I've noticed that I write software in one of three modes:
- For myself: Shortcuts, less testing, not well-factored.
- For myself but in public: Mostly POP Forums, which I try to avoid letting it suck since others will use it and see the code.
- For sharing: Any day job or gig where others will use or maintain your code. You don't want to unleash crapsauce on others.
I have to admit that second case isn't the most clean of endeavors. While I'm generally happy with the forum app and the feedback I get for it, it needs some refactoring in places. The thing that bothers me the most is that a lot of the controllers do way too much. This is particularly obvious because of all the mocking required for the tests. It's not like I've got inline SQL in there, but they could definitely stand to delegate stuff to other stuff.
One of the inevitable things you end up doing at some point in your ASP.NET MVC controller is access the HttpContext in some way. Because the cats that build the MVC framework are so smart, they actually used HttpContextBase as the type for the HttpContext property on the Controller base class. That already makes life a little easier, because you don't have to mock out a huge graph of objects to test what it is your controller is doing.
I suggest that you can take that one step forward. For example, say that you need to molest your model a little before you return it to a view, but only if the client is a mobile browser. It's actually pretty easy to check, but here's the Boolean that tells you:
var isMobile = HttpContext.Request.Browser.IsMobileDevice;
Simple enough, but it's also pretty deep in the object graph for mocking. It also assumes you'll never do any logic more complicated than checking the property. Since you're already using dependency injection in your controller (and if you're not, shame on you, go read up on it), it might be easier to hand off this logic to something else. So in this example, perhaps you have a class called MobileDetectionWrapper, which implements IMobileDetectionWrapper, and looks like this:
public class MobileDetectionWrapper : IMobileDetectionWrapper
public bool IsMobileDevice(HttpContextBase context)
Super simple example, and what might seem like a needless abstraction, but this is far easier to test. Inject the IMobileDetectionWrapper into your controller, and now you simply mock that. Your test might look something like this (using Moq here):
var mobileDetection = new Mock<IMobileDetectionWrapper>();
mobileDetection.Setup(x => x.IsMobileDevice(It.IsAny<HttpContextBase>()).Returns(true);
var controller = new MyController(mobileDetection.Object);
var result = controller.MyActionMethod();
// test the result here for whatever
And just in case you're unclear about the way the controller looks:
public class MyController : Controller
public MyController(IMobileDetectionWrapper mobileDetection)
_mobileDetection = mobileDetection;
public ActionResult MyActionMethod()
var isMobile = _mobileDetection.IsMobileDevice(HttpContext);
// do stuff
Get it? I freehand-typed that without Visual Studio or Intellicrack, so hopefully it's right. :) In any case, the dependency injection resolver you have set up in your MVC app news up the controller with the concrete implementation of IMobileDetectionWrapper and you use it in your action method. Again, trivial example, but imagine you had to do other special things, like check for a cookie and identify an iPad, and therefore return a false value instead of true for the IsMobileDevice question. The necessary mocking in the controller is significantly less.