Showing posts with label DI. Show all posts
Showing posts with label DI. Show all posts

Thursday, February 16, 2012

DIP: Loose or Leaky?


In the last post I looked a bit at the Dependency Inversion Principle which says to define an interface representing the contract of a dependency that you need someone to fulfill for you. It's a really great technique for encouraging SRP and loose coupling.

The whole idea behind the DIP is that your class depends only on the interface, and doesn't know anything about the concrete class implementing that interface.  It doesn't know what class it's using, nor does it know where that class came from.  And thus, we gain what we were after: the high level class can be reused in nearly any context by providing it with different dependencies.  And on top of that, we gain excellent separation of concerns, making our code base more flexible, more maintainable, and I'd argue more understandable.  Clearly, the DIP is awesome!

But!  I'm sure you were waiting for the But...  Since we now have to provide our object with it's dependencies, we have to know:
  • Every interface it depends on
  • What instances we should provide
  • How to create those instances
This feels so leaky to me!  I used to have this beautiful, self contained, well abstracted object.  I refactor it to be all DIP-y, and now it's leaking all these details of it's internal implementation in the form of it's collaborators.  If I had to actually construct this object myself in order to use it, this would absolutely be a deal breaker!

Fortunately, someone invented Inversion of Control Containers, so we don't have to create these objects ourselves.  Or, maybe unfortunately, 'cause now we don't have to create these objects ourselves, which sweeps any unsavory design issues away where you wont see them...

What design issues?  Well, the leaking of implementation details obviously!  Are there others?  Probably.  Maybe class design issues with having too many dependencies?  Or having dependencies that are too small and should be rolled into more abstract concepts?  But neither of these is a result of injecting Inverted Interfaces, only the implementation leaking.

I do believe this is leaky, but I'm not really sure if it's a problem exactly.  At the end of the day, we're really just designing a plugin system.  We want code to be reusable, so we want to be able to dynamically specify what dependencies it uses.  We're not forced to pass the dependencies in, we could use a service locator type approach.  But this has downsides of it's own.

In the next post, I'll wrap this up by zooming back out and trying to tie this all together.

Sunday, February 5, 2012

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.

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.

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

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).

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.

Wednesday, February 1, 2012

Interfaces, DI, and IoC are the Devil

I want to write unit tests. To do that, I need to be able to swap out the "dependencies" of the object under test with mocks, or stubs, or fakes. In static languages like C# or Java this is accomplished by two means:
  1. Using interfaces instead of the concrete class
  2. Injecting instances of those interfaces through the constructor (Dependency Injection)
There are other ways, but this seems to be the widely regarded "best" way.  And I don't like it.

The core of my dislike stems from one simple thing: interfaces don't describe constructors.  So once I'm forced to depend on interfaces, I can no longer use the programming language to create objects.  I can't use the new keyword.  The simplest most fundamental feature of all objects, creating them, is barred from me.

To be clear, this isn't Dependency Injection's fault, nor is it IoC's fault.  It's the interface's fault.  Here's my favorite feature of Ruby:
MyClass.Stub(:new) {...}
That simple line of code stubs out the new 'message' to the MyClass type, allowing me to return a stub instead of an actual instance of MyClass.  This demonstrates why Ruby is so much easier to test than today's static languages:
  1. You can intercept any message to the object w/o having to depend on an interface
  2. You can intercept the constructor the same way you'd intercept any other message
But back to topic, why is it a problem that I can't use the new method?  In my experience this causes some nasty coding practices to emerge.  I'll take them in turn:

Everything is a singleton
As soon as I couldn't create instances, I wrote all of my objects in a stateless style (no instance variables) and registered them as singletons in the IoC container.  This leads to all sorts of other code smells within that object.  Since you can't promote variables to instance variables you end up passing all your state into methods, leading to methods with lots of parameters, which is a Clean Code smell.  Methods passing parameters to other methods to other methods to other methods...

Custom "init" methods are introduced
To combat the last problem, you might add an initialize method to take the place of the constructor.  This will work, though it's non-standard and confusing.  You also have to decide how to register the class in the IoC container.  I've seen this done with a singleton, where the Init method clears the old state and setups the new state WHICH IS EVIL.  Or you can have it create a new instance each time, but you'll never know how it's configured when you're using it, more on that later.

Classes that could benefit from refactoring to have instance variables, don't get refactored
Both of the above contribute to this problem.  When you add that next "context" parameter  to your stateless class, it may be the straw to break the camels back, causing you to refactor those parameters to instance variables.  But time and time again I've seen the uncertainty around the lifecycle of the class in IoC lead people to delay this refactoring.  Again, more on the lifecycles later.

Factory interfaces are created to wrap the constructors
Finally, when I'm fed up with the above, I introduce a factory class who's sole purpose is to wrap the constructor of the class I actually want.  This is just bloat, plain and simple.  And it's yet another class that gets registered with the IoC container.

I also have some complaints about DI and IoC when they are leveraged simply to enable unit testing.  I'd like to call into question some of the typical assumptions around DI/IoC, and we'll see where it takes me.

Interfaces are good because they make your code loosely coupled
This is the most common assumption I see that I disagree with.  I've seen this in Fowler's writing on DI and in the GOOS book, which makes me seriously doubt myself here.  But no amount of time or practice has changed my opinion.  The argument is that it's good to use interfaces over ALL OF YOUR CLASSES because it allows you to make them ALL PLUGABLE.

I find this completely ridiculous!  There are certainly classes that it is useful to have be "plugable" but they are few and far between.  The majority of the classes that I create are Single Responsibility, extracted from other classes, and small.  They serve a very pointed, very well understood, and very well defined purpose in the larger web of objects.  The chances I would want to PLUGIN in a different version of this class are virtually zero (aside from for testing).  I might change it as the design of my domain changes over time, and that will be easy because it's small and focused.  But I'm not going to suddenly wish it was plugable!  I might even argue that wrapping these things in interfaces hurts cohesion.

I should clarify, I do want to swap these classes out so I can test, and that's why I'm forced to use interfaces.  But the argument that using interfaces for their own sake, even if I wasn't testing, for all classes is a good thing is one I just find outrageous.  It's an unnecessary level of indirection, adding more complexity, and giving me nothing useful in return: YAGNI.

IoC is good because it makes the lifecycle of my objects someone else's problem
This is another argument I've frequently seen in favor of IoC.  That moving the responsibility for the "lifecycle" of your objects all the way up to the top of your application is a good thing.  This is another argument I find mind boggling.  Again, there are a minority of cases where this is useful.  An NHibernate session that needs to be created for each web request comes to mind.

But again, 90% of the time I've got an instance of an object that wants to delegate some task pertaining to its current state to some other object.  I want that object to come into existence, perform some task for me, and go away.  Basically, if I wasn't encombered by interfaces and DI and IoC, I'd new up the object passing in the necessary state and tell it to do what I need.  The garbage collector would take care of the rest.  But when I'm burdened by IoC, suddenly there is a lot of uncertainty around what the lifecycle of each object should be.  And you can't tell what lifecycle a given injected object has.  This is a significant issue that affects how you will use that object, and how that object is designed.

Dependency Injection is good because it makes my object's dependencies obvious
I've encountered this argument in many places too, notably this blog post.  The argument roughly goes: "To unit test an object you need to know what it's dependencies are so you can mock them.  But if those dependencies aren't clearly specified in the constructor, how will you know what they are?!  So DI is awesome because it clearly specifies the dependencies!"

This is such nonsense it's really quite amusing.  Knowing what the dependencies are doesn't even BEGIN to help me mock them out.  I have to know what methods are called, with what parameters, expecting what return value, and how many times!  In other words, I have to know everything about how the object uses that dependency to successfully and cleanly mock it out.  A list of what dependencies are used barely scratches the surface and is definitely NOT a compelling reason to use Dependency Injection!

Another downside of IoC/DI that I've often run into is that it spreads like a virus through your code.  Once you start using DI, every class ABOVE that class in the call stack also has to use DI.  Unless you're OK with using the container as a service locator, which few people are.  So essentially, if you're going to use DI you're going to have it in the majority of your classes.

I'm painfully aware that this post is just a lot of complaining and contains no useful suggestions toward correcting any of these issues.  What I want is what Ruby has, but C# just doesn't support that.  I may experiment with some forms of service locator, leveraging extension methods.  Or I might try to create some sort of internally mockable reusable factory concept.  I'll write it up if it leads anywhere, but I doubt it will.

So I guess the only point of this post is to vent some of my frustrations with DI, IoC, and interfaces and get feedback from all of you.  Perhaps you have felt some of this pain too and can validate that I'm not totally crazy?  Or maybe, and I sincerely hope this is true, I'm missing some fundamental concept or perspective on this and you can fill me in?  Or might it even be possible that this could be a first step toward cleaning up some of these pain points, even if just in a small way?

Or you might read all of this and shrug it off.  You're used to it.  It's always been this way, it can't be any other way, go about your business.  If that's your thinking as you're reading this sentence, I'd just encourage you to re-examine your assumptions about what's possible.  Take a look at Bernhardt's testing in DAS.  THEN come back and give me a piece of your mind.