Thursday, July 19, 2012

If you write an article about TDD, make sure it is correct

I spend a majority of my private time reading articles and blog posts, having discussions on Twitter, or engaging in conversations on conferences and community events. I'm realistic enough to understand that my opinions are not necessarily the truth, so I use those opportunities to challenge my own ideas and learn about solutions I would never think of myself.

I recently read an article titled Test-Driven ASP.NET MVC that, although I respect what the author is trying to convey, implies some things that I strongly believe are incorrect or are malpractices. Normally I wouldn't bother to blog about this, but because his article is published at the online MSDN Magazine, I couldn’t resist the urge to correct some things.

  • Thoroughly understand the MVC pattern. As Martin Fowler has clearly explained in his post on GUI patterns, the view doesn't manage the presentation of models, nor does it handle interactions with the users. Both are the responsibility of the controllers. They react to keyboard, mouse (and touch) input, use that information to interact with the (domain) model and then decide what view should be used to render the result. In that sense, ASP.NET MVC is reasonably faithful to the original pattern. The only big difference is that in the original MVC pattern, the controllers were directly handling user input whereas in ASP.NET MVC the controllers merely handle HTTP POSTs, GETs and PUTs.
  • Tiers are not layers. Tiers are used to represent a physical separation whereas layers represent a logical separation. You can have all layers (e.g. presentation, service, domain and data access) hosted on a single tier if you want, or you can assign them to multiple tiers. An example of that is an ASP.NET web site (the presentation layer) hosted on a front-end web server, the service, domain and data access layers hosted on a Intranet application server, and the database hosted on a dedicated database server. The other way around doesn't work like that though. If you didn't account for layering in your architecture, it might be very difficult to deploy your system in a distributed environment.
  • Keep your Visual Studio projects to a minimum. Having many projects such as the project-per-layer suggested in the article considerably slows down compiling, but also causes the startup time of the corresponding application to increase. Loading 10 small assemblies is much slower than loading one big assembly. Jeremy D. Miller mentioned this already in this post, but Patrick Smacchia, the author of NDepend, delivered some proof of if it here. Read my original post for more guidance on splitting up projects.
  • Separate test code from production code. The author recommends separating test code from production code, a good thing obviously. In fact, I would not even dare to think of polluting the production code base with test code. It might double the size of the assemblies and considerably increase the memory footprint of the application. But as usual, it is not always possible to avoid this. We sometimes have to make a method internal and allow the test projects to access those members using an [InternalsVisibleTo] assembly-level attribute. But that's an exception. What we do more often is make certain methods protected so that we can apply the Test-Specific Subclass pattern.
  • TDD is about test-first development. The article refers to the Test-Driven Development practice a few times but never explains its essential concepts such as writing tests before writing the production code, the red-green-refactor mantra and the many benefits it offers from a design and maintenance perspective. And neither does it explain what the exact meaning of a unit in unit testing is. In fact, not a single code example shows what a unit test would look like when practicing TDD in an ASP.NET MVC application. Why then call the article Test-Driven ASP.NET MVC. This is even more surprising considering that the author is using Jeremy D. Miller's StructureMap, somebody who has been one of the most noticeable TDD advocates in the community. I wonder whether the author really understands TDD at all.
  • Use Fakes with caution. This is also amplified by his apparent nonchalant use of Visual Studio 2012's support for generating fakes from any class. If you're practicing TDD, you’ll explicitly design the interface of your API or type and think about its dependencies. IMHO, this is an essential part of the thought process involved in TDD. Using a brute-force tool like Fakes or TypeMock Isolator allows you to ignore that part of the process and just generate whatever is needed for your test. I'm not saying Fakes or TypeMock are bad products, but because of their unique capabilities compared to popular mocking frameworks like Moq, Nsubstitue or my own favorite, FakeItEasy, I'm afraid many people will follow the author's example.
  • Dependency injection is not the same as the inversion of control principle. I hate it when developers use the terms Dependency Injection and Inversion-of-Control interchangeably. They are two different things and if you don't understand that, make sure you thoroughly read the chapter on the Dependency Inversion Principle in Pablo's SOLID guidelines. Although subtlety different, Inversion-of-control is essentially about removing the direct dependencies of one class to another out of that class and providing an abstraction that that class relies on. The inversion in this principle is about moving the responsibility for hooking up the original class with a concrete implementation of that abstraction to a 3rd party, typically an IoC framework. This will improve your chances for loose coupling, better testability, and increased maintainability on the long run. Obviously you don't have to use a DI framework, but if you do it correctly you can gain a lot of benefits from it. I myself heavily rely on Autofac after having used Microsoft Unity for a few years. And remember, just like any principle or tool, use it with caution. Overusing IoC can result in systems were it is difficult to understand what happens.
  • Resolving dependencies from inside a class completely defies the ideas behind DI. Why in the world would you suggest people to use an DI framework like StructureMap and then use a static class to resolve dependencies from within the dependent class? That vaporizes the entire purpose of a DI framework. It's not for nothing that Jeremy D. Miller wrote a post on Push, Don't Pull. Even better, that's why the word injection is in Dependency Injection! That is also why the author suggest registering a fake object in his IoC container. This is clearly a design smell resulting from his apparent incorrect understanding of the IoC principle.
  • One more issue. In the discussion on namespaces and test code, the author suggested to use the name Test (singular) in the project name. Since the namespaces in projects should use the project name as their base, you end-up with the name of something that could be the name of a class. That has been a violation of the Microsoft Design Guidelines and FxCop naming rules since the inception of the .NET Framework. I usually use the name Tests, Testing or Specs for the project that holds the unit and integration tests.

Anyway, the morale of this post is that you should get your facts straight before you write an article that is going to reach so many people. And now that I think of it, why didn't somebody from the MSDN Magazine's staff check this article in the first place? I might be wrong as well, so if disagree let me know by commenting on this post or by pinging me through my twitter handle at ddoomen.

5 comments:

rliesenfeld said...

Great article! I mostly agree, but there are some inaccuracies:

1) TDD does not require the creation of *unit* tests, where dependencies are mocked. You can very well only write integration tests while practicing TDD. So, TDD doesn't actually tell you to design the interface of dependencies in your tests.

2) Inversion of Control is not the same as the Dependency Inversion Principle (DIP). (And DI is yet something else; *three* very different things.)

3) DI may be useful, but the Service Locator pattern can often be a better/simpler choice. My point here is that all too often "DI" becomes simply an excuse to adopt a big "application framework", giving rise to lots of ultimately useless XML code and pointless separate interfaces.

Dennis Doomen said...

Hi rliesenfield,

Thanks for the appreciation and the feedback.

I'm quite sure I never specified the scope of a unit in a unit tests. Our units are usually a combination of a few classes so that we have a good balance between maintainability and refactorability. We even have integration tests that touch a database, file system or other expensive resource. For me a unit test is just an automated test that is extremely fast, runs in-memory and doesn't have any side-effects. The dependencies part usually happens without thinking because you want your classes to be easily testable. Refactoring dependencies behind an interface is an easy solution for the need to introduce a backdoor (see XUnit Patterns for more examples).

I think you're right about the subtle difference between DIP and IoC, though I strongly believe that the Service Locator pattern is an anti-pattern. But I do agree that there isn't a silver bullet and careful consideration of the pros and cons is required before choosing a particular tool, framework or pattern.

Anton Boichev said...

Hi all,

Thanks for sharing your thoughts on this, I find it very helpful for me.

Original msdn magazine author says in the article: "Larger applications generally have too many dependencies to supply them via the object’s constructor."

From your experience, what do you guys think is the best approach when real world objects have lots of dependencies? Is this a problem for constructor based injections?

Thanks!

Dennis Doomen said...

If a class has more than three dependencies, you might have a problem anyway. Hadi Harari (a JetBrains product manager) wrote a good blog post about that. See http://hadihariri.com/2012/04/09/dealing-wht-the-too-many-dependencies-problem/

Andrew Beaven said...

"Why then call the article Test-Driven ASP.NET MVC"

It's not - the title of the article is "Test-Driving ASP.NET MVC". I don't think that the purpose of the article was to necessarily explain TDD so the point is moot. Other than that, I agree with everything else :)