Wednesday, 10 March 2010

Fear of the new (how TDD and DI encourage bad practice)

When I first started doing TDD back in the mid-naughties it changed the way I programmed dramatically.  It took me a while to get it at first, how can you test one unit? until I discovered the tricks of dependency injection.  Now my code was shaped in loosely-coupled wonderness and I felt invincible.  I thought I had reached the pinnacle of OOP. Everything suddenly became so easy, it was well tested, easy to change, easy to wrap, swap implementations, decorate, whatever, I was throwing the best design patterns out with such ease that I barely needed to refer to the Gang-of-Four anymore.

I went on in this vein for a good year or so.  And as the millenium's first decade entered early evening and its bright lights began to fade I took a step back and looked at my code.  My god, I thought, this is terrible, what the hell have I been doing? And it was terrible, hundreds of interfaces named somebody's Manager, Persister, Service (oh so many classes called Service), Validator etc. etc., all with a single namesake class implementing them.  There were tens of classes whose purpose seemed to be to co-ordinate these little buggers - for they were little, ten or so lines each (oh how I thought I was doing a grand job) - and they would hand off to other similar classes who co-ordinated other classes and it was turtles all the way down.  And in amongst all of it there were about a dozen classes that had real names like Customer, Account (but even they had interfaces too) and, aye how the sting of memory pains me to this day, they only had getters and setters (it's the shame, you never get over the shame).

It was with great pain I realized for the last year of my professional life I had been producing procedural code with all the idealistic misguided joy of a card holding Russian worker pouring cheap cement to build pointless roads.  I knew the culprit alright: TDD and DI.  I had gone decoupling, unit testing mad.  The result was an irrational fear of the new keyword.  As far as I was concerned the new keyword was banished to the dark depths of Containers, it was a clumsy language feature never to be used again.  But what I had instead was a dependency graph made up almost entirely of stateless singletons.  A simple Ruby script could have replaced every injected dependency with calls to static methods and sure the application would have lost its testability, it would no longer be decoupled, but essentially, at it's essence, and in terms of structure, it would be the same.  TDD and DI had simply given me a fancy way of decoupling static classes.

The new keyword is a wonderful thing.  It lies at the heart of OOP.  Its sole objective is to create a new instance of an object.  If you don't use new you can't create new objects and if you can't create new objects you can't write object orientated code.  And the other fundamental thing about objects is they encapsulate behavior and state.  Singletons don't.  Singletons are procedural.  Regardless of whether they are loosely coupled and injectable.  Real OO classes have private fields (the fewer the better) and methods which do things based on those private fields, sometimes based on arguments passed in, but always, always in the context of those private fields.  And the only way, the one single true way you get values into those private fields is via the constructor and that means using the new keyword.  Let's put it clearly: this 'new ValidatedCustomer(customer).IsEmailAddressValid' is OOP. This 'customerValidator.Validate(customer).IsEmailAddressValid' is procedural.

Now I was writing code using TDD and DI that was object orientated code.  But my fear of the new was still there.  In order to maintain the injectable, loosely-coupled gorgeousness I had begun to do a horrible thing.  I started injecting factories everywhere.  Sure it was an improvement but there was still something horribly smelly going on.  After all the factories were essentially static, singletons whose sole purpose was to delegate to the new statement.  I mean there's single responsibility and then there's craziness!

So I started doing something I thought was really bad: I created objects in my classes! But I wanted to keep loosely coupled and all of that wonderfulness.  This required a dramatic shift in thinking.  Remember when you first did TDD and how it really hurt because you had to structure programs in a different way?  Well it was like doing that all over again.  Every time I started doing something I had to spend half an hour thinking, how do I make this work?  I know I'm right but how?

I learnt a few lessons:
  • Not all classes are about interactions, sometimes they are about results.  Mocking everything out for the sake of it doesn't gain you anything.  Use state based testing to assert the end results not interaction testing to assert what is happening to achieve those results.  
  • It's OK for your collaborators to new up something if you ask them, so you can say fileSystem.NewFile(name) if you need to.  
  • Collaborators don't always have to be passed in the constructor: they can be passed as arguments to methods as well, so you can say new File('myfile.txt', 'Some text').SaveTo(filesystem).  
  • If a class is unavoidably all boilerplate and co-ordination consider using integration tests, after all mocking the interactions of boilerplate code doesn't tell you anything useful at all.  
  • The container shouldn't contain everything, it should be the things that generally require configuration or are likely to change: databases, filesystems, web services etc. Rarely do core domain classes or business principles need to be 'switchable'.

12 comments:

Chandan said...

Agree, about the "abuse" of interfaces and DI,. We do it in name of "what if" customer wants to extend the functionality, for e.g. in an app we provided "MetricWriter" interface and Impl class even though we knew its only going to be a DB write --why cannot we just provide a concrete for DB writing --am confused :(?

sud said...

You can certainly go overboard with any particular technology or technique. One has to always be pragmatic about any choosing the right technology, technique, design etc. based on the need and specific scenario.

Having said that, I still favor injecting my dependencies when I need to be able to mock or stub the dependency for my test case even though we may have only one implementation of that dependency ever. The value of having a test that can avoid any regression defects outwieghs the one time programming cost of a decoupled design.

The other suggestion about switching a unit test that mocks out its collaborators to an integration test when all it does is co-ordination doesn't appeal to me. I do not want to have to verify the success or failure of the test by verifying the result in the external system. Using a mock allows me to simply verify that the right message was sent to the external system and still have a safety to prevent regression defects.

Sidu said...

I'm missing something here. I don't see how you could create a Manager class and still have single responsibility - and this has little to do with TDD or DI. Being particualar about OO guidelines while doing TDD should automatically prevent the creation of Managers and Validators (which can't possibly have state, but aren't blocks). Having an interface implemented by a single class is just waste, in the lean sense of it. Any halfway decent IDE will allow you to run an extract interface refactoring when you actually have a second entity that shares a public contract with the first.

Peter Gillard-Moss said...

@Chandan and @Sidu: Agree that creating interfaces for single implementors is a waste. You don't need an interface to mock: you can provide 'abstract' methods which are overridable by the mocking framework. From there it's a couple of clicks to introduce an interface if you need to.

@sud: I am not proposing we sacrifice testability or loose coupling. In terms of integration tests I mean an integration of a small number of units (2-3) all external systems should be mocked out. e.g. if you have a class which co-ordinates one class which loads XML from a filesystem and another that desieralizes it to an object don't fear newing up the deserizlier but do still mock the filesystem.

@Sidu: I agree. All those things are bad. That's my point. But I'm illustrating from my own experience how a misguided application of TDD and DI led me to believe I was achieving good design etc. Further we agree singleton classes are bad, but google DI and you'll find most of the examples employ these extensively in their design (even Mr Fowler's - MovieLister and MovieFinder). See why I (and others) have been so misled?

Anonymous said...

"Agree that creating interfaces for single implementors is a waste."

My poor analogy:

A shopkeeper opens a coffee shop. It is new, no word of mouth or advertising has gotten out but there is one friend and regular customer called Bill. So, when an item is ordered, the shopkeeper new's up a Bill, records the sale and everything goes ok for the first month.

But darn it, someone else other than Bill comes in and places an order. So the shopkeeper new's up a Bill and...

That's not right. You are not dealing with a Bill but a Customer (or ICustomer if you must). Using 'new' can also lead you down a similar bad road because you should be dealing with a role and not a concrete someone.

My point is that creating an interface for a single implementor can often not be a waste because it can force you to think about and realize that you are dealing with a role. Avoiding interfaces with only a single implementor can help you paint yourself into a corner when you should be thinking about roles.

Peter Gillard-Moss said...

Anonymous, in your analogy you are quite correct, there is a need for an interface. Where an interface is needed it isn't wasteful. However interfaces are not needed all the time.

Pramod Negi said...

Nice blog. Interesting information.
This looks like a very nice place! I liked it so much and very interesting, too! Thanks for sharing the experience.
Cheap Flights to Hanio
Flights to Hanio
Hanio Flights

radhe said...

I recently came across your blog and have been reading along. I thought I would leave my first comment. I don't know what to say except that I have enjoyed reading. Nice blog. I will keep visiting this blog very often. Very informative content and I enjoyed reading your blog keep it up.
thailand Holiday Packages
Cheap thailand Holidays
thailand Holidays

Veer said...

Nice blog. Interesting information.
This looks like a very nice place! I liked it so much and very interesting, too! Thanks for sharing the experience.
barbados Holiday Packages
Cheap barbados Holidays

Anonymous said...

The same reason that people buy Youtube views is why they buy Facebook
fans. Google Calendar gives you an ability to customize your view as you like.
Also, economic growth is unlimited in the world and
should not be restricted by worry about natural resources.



my web blog ... youtube video views

Jessica Jeon said...

To have an increased publicity for your facebook account, People expend bucks to Buy Facebook Followers to easily get exposure on web. buy followers for fb

Abhijit said...

To acquire a higher publicity to your products or personality, People expend cash to Buy Facebook Comments to quickly obtain exposure immediately. buy facebook post comments

About Me

My photo
West Malling, Kent, United Kingdom
I am a ThoughtWorker and general Memeologist living in the UK. I have worked in IT since 2000 on many projects from public facing websites in media and e-commerce to rich-client banking applications and corporate intranets. I am passionate and committed to making IT a better world.