Demonstrating the Costs of DI

Here's a followup to my previous post "Interfaces, DI, and IoC are the Devil"

This time I want to demonstrate a little of what I'm talking about with some code samples.  I have contrived an example.  I wanted something that is real, so I took the code structure and the responsibilities from some real code in one of our applications.  But I changed the names to make it more easily understood.  I also removed some of the concerns, such as transaction handling and record locking, just to make it shorter.  I'm trying to be fair, so the example is not trivial, but it is also not complicated.

Note that I didn't compile any of this code, so please excuse typos or obvious syntactic errors I might have over looked.

Pretend we are writing an application to manage a conference.  It's an MVC 3 C# web app.  We have presentations, and presentations have speakers.  The speakers are ordered (1,2,3, etc).  As an optimization, we will cache some of the information about the speakers for a given presentation: how many are there and who is #1 ordered speaker.  This information will be cached on the PresentationSpeakers table.

The structure of the code is as follows.  The SpeakerController's Create action method is called to add a new speaker to a presentation.  This controller delegates the job to the PresentationApi class.  This class deals with coordinating the various domain services, and in real life would have dealt with database transactions and record locking/concurrency.  PresentationApi delegates to OrdersSpeakers (which obviously assigns the order numbers to the speakers) and PresentationSpeakers (which caches information about the speakers on a presentation).  Finally the Speaker and Presentation classes are active record objects.

This first example demonstrates the simplest way in which I would like to write this code.  There are no interfaces, there is no constructor injection, and it is using the Active Record pattern for database persistence.

	public class SpeakerController : Controller
	{
	public ActionResult Create(int presentationId, FormCollection data)
	{
	var presentation = Presentation.Find(presentationId);
	var vm = SpeakerNewViewModel.Build(presentation);
	UpdateModel(vm, data);
	var presentationApi = new PresentationApi(presentationId);
	presentationApi.AddSpeaker(vm.Speaker);
	return View();
	}
	}
	public class PresentationApi
	{
	int presentationId;
	PresentationSpeakers presentationSpeakers;
	OrdersSpeakers ordersSpeakers;
	public PresentationApi(int presentationId)
	{
	this.presentationId = presentationId;
	ordersSpeakers = new OrdersSpeakers(presentationId);
	}
	public void AddSpeaker(Speaker speaker)
	{
	LoadPresentationSpeakersRecord();
	speaker.PresentationId = presentationId;
	presentationSpeakers.AddSpeaker(speaker);
	ordersSpeakers.AddSpeaker(speaker);
	speaker.Save();
	presentationSpeakers.Save();
	}
	public void RemoveSpeaker(Speaker speaker)
	{
	// ... very similar to AddSpeaker ...
	}
	void LoadPresentationSpeakersRecord()
	{
	presentationSpeakers = PresentationSpeakers.FindByPresentationId(presentationId);
	}
	}

view raw 1-Simplest.cs hosted with ❤ by GitHub

The next example addresses the "problem" of the controller having a direct dependency on the PresentationApi class by adding IPresentationApi and injecting it through the controller's constructor. Notice that I also convert PresentationApi to be a singleton at this point and remove it's instance variables. This isn't strictly required, but it is typical. Notice how I now have to pass the presentationId into the LoadPresentationSpeakersRecord helper method.

	public class Modules
	{
	public void Register(IContainer container)
	{
	container.Register<IPresentationApi>.To<PresentationApi>();
	}
	}
	public class SpeakerController : Controller
	{
	IPresentationApi presentationApi;
	public SpeakerController(IPresentationApi presentationApi)
	{
	this.presentationApi = presentationApi;
	}
	public ActionResult Create(int presentationId, FormCollection data)
	{
	var presentation = Presentation.Find(presentationId);
	var vm = SpeakerNewViewModel.Build(presentation);
	UpdateModel(vm, data);
	presentationApi.AddSpeaker(presentationId, vm.Speaker);
	return View();
	}
	}
	public interface IPresentationApi
	{
	void AddSpeaker(int presentationId, Speaker speaker);
	void RemoveSpeaker(int presentationId, Speaker speaker);
	}
	public class PresentationApi : IPresentationApi
	{
	public void AddSpeaker(int presentationId, Speaker speaker)
	{
	var presentationSpeakers = LoadPresentationSpeakersRecord(presentationId);
	speaker.PresentationId = presentationId;
	presentationSpeakers.AddSpeaker(speaker);
	var ordersSpeakers = new OrdersSpeakers(presentationId);
	ordersSpeakers.AddSpeaker(speaker);
	speaker.Save();
	presentationSpeakers.Save();
	}
	public void RemoveSpeaker(int presentationId, Speaker speaker)
	{
	// ... very similar to AddSpeaker ...
	}
	PresentationSpeakers LoadPresentationSpeakersRecord(int presentationId)
	{
	return PresentationSpeakers.FindByPresentationId(presentationId);
	}
	}

view raw 2-WithIPresentationApi.cs hosted with ❤ by GitHub

In the third example, I remove PresentationApi's direct dependency on OrdersSpeakers.

	public class Modules
	{
	public void Register(IContainer container)
	{
	container.Register<IPresentationApi>.To<PresentationApi>();
	container.Register<IOrdersSpeaker>.To<OrdersSpeaker>();
	}
	}
	public class SpeakerController : Controller
	{
	IPresentationApi presentationApi;
	public SpeakerController(IPresentationApi presentationApi)
	{
	this.presentationApi = presentationApi;
	}
	public ActionResult Create(int presentationId, FormCollection data)
	{
	var presentation = Presentation.Find(presentationId);
	var vm = SpeakerNewViewModel.Build(presentation);
	UpdateModel(vm, data);
	presentationApi.AddSpeaker(presentationId, vm.Speaker);
	return View();
	}
	}
	public interface IPresentationApi
	{
	void AddSpeaker(int presentationId, Speaker speaker);
	void RemoveSpeaker(int presentationId, Speaker speaker);
	}
	public interface IOrdersSpeakers
	{
	void AddSpeaker(int presentationId, Speaker speaker);
	void RemoveSpeaker(int presentationId, Speaker speaker);
	}
	public class PresentationApi : IPresentationApi
	{
	IOrdersSpeakers ordersSpeakers;
	public PresentationApi(IOrdersSpeakers ordersSpeakers)
	{
	this.ordersSpeakers = ordersSpeakers;
	}
	public void AddSpeaker(int presentationId, Speaker speaker)
	{
	var presentationSpeakers = LoadPresentationSpeakersRecord(presentationId);
	speaker.PresentationId = presentationId;
	presentationSpeakers.AddSpeaker(speaker);
	ordersSpeakers.AddSpeaker(presentationId, speaker);
	speaker.Save();
	presentationSpeakers.Save();
	}
	public void RemoveSpeaker(int presentationId, Speaker speaker)
	{
	// ... very similar to AddSpeaker ...
	}
	PresentationSpeakers LoadPresentationSpeakersRecord(int presentationId)
	{
	return PresentationSpeakers.FindByPresentationId(presentationId);
	}
	}

view raw 3-WithIOrdersSpeakers.cs hosted with ❤ by GitHub

Finally I eliminate ActiveRecord and replace it with the most evil pattern in the world, the Repository pattern. I chose to implement this in the way I've most commonly seen, with one interface per database table (give or take).

	public class Modules
	{
	public void Register(IContainer container)
	{
	container.Register<IPresentationApi>.To<PresentationApi>();
	container.Register<IOrdersSpeaker>.To<OrdersSpeaker>();
	container.Register<IPresentationRepository>.To(...);
	container.Register<IPresentationSpeakersRepository>.To(...);
	}
	}
	public interface IPresentationRepository
	{
	...
	}
	public interface ISpeakerRepository
	{
	...
	}
	public interface IPresentationSpeakersRepository
	{
	...
	}
	public class SpeakerController : Controller
	{
	IPresentationApi presentationApi;
	IPresentationRepository presentationRepo;
	public SpeakerController(
	IPresentationApi presentationApi,
	IPresentationRepository presentationRepo)
	{
	this.presentationApi = presentationApi;
	this.presentationRepo = presentationRepo;
	}
	public ActionResult Create(int presentationId, FormCollection data)
	{
	var presentation = presentationRepo.Find(presentationid);
	var vm = SpeakerNewViewModel.Build(presentation);
	UpdateModel(vm, data);
	presentationApi.AddSpeaker(presentationId, vm.Speaker);
	return View();
	}
	}
	public interface IPresentationApi
	{
	void AddSpeaker(int presentationId, Speaker speaker);
	void RemoveSpeaker(int presentationId, Speaker speaker);
	}
	public interface IOrdersSpeakers
	{
	void AddSpeaker(int presentationId, Speaker speaker);
	void RemoveSpeaker(int presentationId, Speaker speaker);
	}
	public class PresentationApi : IPresentationApi
	{
	IOrdersSpeakers ordersSpeakers;
	ISpeakerRepository speakerRepo;
	IPresentationSpeakersRepository presentationSpeakerRepo;
	public PresentationApi(
	IOrdersSpeakers ordersSpeakers,
	ISpeakerRepository speakerRepo,
	IPresentationSpeakersRepository presentationSpeakersRepo)
	{
	this.ordersSpeakers = ordersSpeakers;
	this.speakerRepo = speakerRepo;
	this.presentationSpeakerRepo = presentationSpeakerRepo;
	}
	public void AddSpeaker(int presentationId, Speaker speaker)
	{
	var presentationSpeakers = LoadPresentationSpeakersRecord(presentationId);
	speaker.PresentationId = presentationId;
	presentationSpeakers.AddSpeaker(speaker);
	ordersSpeakers.AddSpeaker(presentationId, speaker);
	speakerRepo.Save(speaker);
	presentationSpeakerRepo.Save(presentationSpeakers);
	}
	public void RemoveSpeaker(int presentationId, Speaker speaker)
	{
	// ... very similar to AddSpeaker ...
	}
	PresentationSpeakers LoadPresentationSpeakersRecord(int presentationId)
	{
	presentationSpeakerRepo.FindByPresentationId(presentationId);
	}
	}

view raw 4-WithRepositories.cs hosted with ❤ by GitHub

So, if you had your choice, which of these versions would you prefer to have in your code base? Which would you prefer to have to debug, understand, and maintain?

My answer, not surprisingly, is #1. Notice how each version adds more and more cruft into the code and obscurs what the code is actually trying to accomplish.  This is the cost of indirection.  It's really the same reason people like to fly the YAGNI flag.  And perhaps you've heard the old adage, "Do the simplest thing that could possibly work".  I desperately want as few constructs between me and what my code does as possible.  Which is why I yearn for the simple code in example #1.

PS.  If you actually read those code samples and studied the differences between them, pat yourself on the back!  I know this is very difficult to follow.  I actually wasn't intending to blog it, I just wanted to go through the motions myself and see how it turned out.  There have been quite a few times where I wrote something off because of my image of how it would turn out.  But then when I actually did it, it turned out much differently (MVC, MVP, and MVVM were all like that for me).  But in this case, it turned out just how I'd imagined it...  Crufty.