Ayende @ Rahien

Refunds available at head office

The Production Value of Seams

There are some interesting points of views about testable design that I somehow missed catching in real time.

Roy Osherove:

...in many ways, pure object oriented design does not go well hand in hand with the notion of testable design.

He then goes on to show what happens when you addTestability concerns to the some of the usual OOD principals.

Eli Lupian:

Roy and I are on the opposite sides of this methodology. I personally hate to change my production code just for the tests.
As Roy says “pure object oriented design does not go well hand in hand with the notion of testable design.” After creating a good design (without taking testability into account), there is no real business value of implementing hooks to change internal private members and to expose properties that should remain private by design! Unless you want to clutter your code with load of Totally Irrelevant Code
Please, Stop Designing for testability!!

He then shows how you can keep traditional OOP approach and still test your code using TypeMock.

I am firmly in Roy's camp when it comes for designing code to be testable. Before I goes on to explain further, I want to take issue with a couple of points in Eli's post:

Have you ever tried to browse a code with loads of interfaces, it take ages because you have to keep finding the concrete implementation, and the place where the instance was created.

Ctrl+Alt+B in Resharper will take me directly to the implementation. grep *.cs -S "interface\s+INameOfInterface" will do the same if I don't have Resharper. Not an issue.

Think about this code smell: When you find that an interface and implementation have exactly the same public methods, it is a sign that the interface is not exposing a trait of the object but the Object itself

No a code smell at all, in my opinion, just good seperation of the interface and its implementation. I may want to supply alternate implementation, which would be much harder if I had to a concrete class to work with.

Let me share a story. A while ago I was working on a system that needed to write to a file. The simplest code that I could write was:

File.WriteAllText(filename, text);

That is not testable without using an invasive approach like TypeMock, so I choose this way:

using(TextWriter writer = IoC.Resolve<IFileWriterFactory>().Create(filename))
  writer.Write(text);

I added two level of abstractions to the code, just to make it testable. Following Eli's approach, I had decreased the readability of the code and made it harder to understand and maintain. The problem is that I don't think that Eli considered the value of the seams that are created by writing testable code. (Take a look at the stats here, it was a bit more involved).

A seam is a core concept in Working Effectively With Legacy Code. A seam is a place where you can alter behavior in your program without editing in that place. They are crucial for being able to test your code. (Just to note, TypeMock create seams all over the place by simply being able to intercept any call under .Net).

Seams makes the code testable, but it also makes it a lot more flexible. Couple of weeks after I introduced the IFileWriterFactory, I had learned that the files that I was creating were to be consumed by a stupid batch process, which would copy them over to a mainframe system. If I happened to be in the middle of creating the file when the process start running, the mainframe would get a partial file, and chaos would enuse.

The standard way of handling this is to actually output two files, one with the real content, the second with an agreed upon extension, which basically tells the process "if you see a file with extension foo.csv.done, I have finished writing the file foo.csv". I had couple hundreds places in the application that I was writing files.

If I was following the traditional approach, I had to find all the places that I was writing to a file, and change the to:

File.WriteAllText(filename, text);
File.WriteAllText(filename+".done", "");

As I mentioned, there were quite a few place that did it. Because I had a seam in place, I could simple change the returned implementation of the TextWriter that FileWriterFactory implemented, which would generate the ".done" file when the writer was disposed. You can bet that this was a valuable thing to be able to do.

In general, I find that testable design if far more flexible, because it means that you can inject different behaviors into the code. This means that you get more extensible code that you can later leverage to produce more business value. The simple abstraction that I have shown above is something that saved me two days of mindlessly going through the code and making the same stupid change all over the place.

Theoretically, I could probably use some black magic and intercepted the call to FileSteam.Close() and do the same. I do not feel that it would be a maintainable or discoverable solution at all.

So no, I do not believe that testability is not a first class concern, and I feel that it produce much better code in the end. Having more places where you can change the behavior without changing the code is a good thing, and it would end up saving you quite a bit of time. I can see it in Rhino Mocks all over the place, the fact that it is as decoupled as that means that I can take it in different directions (and offer extensability points for users) very easily.

The other side is apperant in many recent Microsoft releases, where internal and sealed are slapped on just about anything, and extensability points for something that they didn't explicitly planned are simply non-existant. This in turn greatly reduce the ability to work with the code.

Comments

Tomer Gabel
03/04/2007 02:09 PM by
Tomer Gabel

It's all a question of compromise between code conciseness and the ability to inject different code or behavior. Coming from a more performance-oriented, systems-programming background, I prefer the former approach. I think that for anyone not well into TDD, the code sample above (with the IoC container) is very difficult to understand or follow. A more traditional (and not necessarily a less powerful one) would be to simply provide a factory method in a utility class; I think

using(TextWriter writer = Helpers.CreateFile(filename)) is much easier to follow than using(TextWriter writer = IoC.Resolve().Create(filename))).

Ayende Rahien
03/04/2007 02:17 PM by
Ayende Rahien

I understand your point, but Helpers.CreateFile() is still not really testable, is it?

And you are talking to someone who is using 4 layers of abstractions just to get a value from a text box. :-)

Thomas Eyde
03/04/2007 07:26 PM by
Thomas Eyde

My naive approach would be to introduce a BatchFileWriter and put all calls to File.WriteAllText in there.

Ayende Rahien
03/04/2007 08:33 PM by
Ayende Rahien

Would you think to do it at first if it was something that wasn't driven by the need for testability?

Eli Lopian
03/04/2007 08:39 PM by
Eli Lopian

Oren,

"I MAY want to supply alternate implementation"

Sounds Like "You Aint Gonna Need It"

See here: http://www.elilopian.com/2007/03/04/design-and-testability-yagni/

Ayende Rahien
03/04/2007 08:55 PM by
Ayende Rahien

Nope, the ability to supply an alternate implementation is a side benefit to the testability benefits.

I would agree with YAGNI, except that this is something that I do need, in order to decrease coupling and enable better testing.

Intersting post, I'm going to reply in a new post.

Thomas Eyde
03/05/2007 08:31 AM by
Thomas Eyde

In response to Ayende:

" Would you think to do it at first if it was something that wasn't driven by the need for testability?"

I can't answer that. I am so used to TDD so testability could be a subconscious desicion. Or it could be that TDD also taught me how to code better.

A BatchFileWriter would of course be testable, but my main reason, I think, is to encapsulate the behavior.

Anders Nor&amp;#229;s' Blog
03/05/2007 01:09 PM by
Anders Nor&#229;s' Blog

There is an interesting discussion going on between Eli Lopian , Roy Osherove and Oren Eini on whether

Comments have been closed on this topic.