Balancing maintainability, readability and testability

Published 04 March 07 08:09 PM | andersnoras 

There is an interesting discussion going on between Eli Lopian, Roy Osherove and Oren Eini on whether or not you should design your code for testability. Eli ended one of his posts with a question about an example Oren gave in one of his posts.

Oren’s example was all about having the benefits of designing for testability. Rather writing his IO code in the simplest way possible, he introduced a factory to resolve a TextWriter.

File.WriteAllText(filename, text);

 

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

This is a classic example of keeping a healthy balance between your “itites”. The first snippet gets a high score in simplicity and readability, but it fails on extensibility and testability. The other snippet is very testable since you can mock the IFileWriterFactory, and it is extensible because the client code makes no assumptions about what sort for TextWriter it uses. This flexibility comes at the expense of readability and “recognizability”. A developer who is not familiar with inversion of control will have a hard time understanding the second snippet, while anyone will “get” the first one.

We should be careful about mixing up maintainability and readability. While it is of utmost importance that the code can be read, it is also important that one can change or extend it during maintenance. A common argument for test driven development is that it improves the design, and extensibility is one of the nice byproducts of testable object oriented programming. I agree with Eli that you shouldn’t design for testability for the sake of it, and I believe that Oren’s seemingly complex design for a simple task grew from a bigger need. If you have a simple application that just emits a file using the simple File.WriteAllText approach is sufficient. You can often test this by running the code and inspecting the created file, however when the complexity of the solution grows the testability requirements follow.

Even if you can use tools like TypeMock to mock the implementation of the File class if the testing needs grow I strongly believe that you should change your design for two reasons: It is often tedious to mock concrete framework classes such as System.IO.File. I once had to write a test fixture around framework that used the System.Data classes directly. TypeMock allowed me to mock low-level types like SqlConnection, but the effort required to do this is huge. Rather than making the tests depend heavily on frameworks, I rather address these dependencies to make the code easier to maintain and the tests easier to write.

The other reason is that the need to mock concrete types is an indication of too strong coupling in the design. Using inversion of control, factories or other means of abstraction makes the implicit dependencies explicit and the code becomes easier to maintain.

Even if there is a connection between readability and maintainability, you often have to make trade-offs on these. I believe that it is a good practice to optimize the whole rather than doing micro optimizations. An extensibility optimization might reduce readability for some passages of the source in exchange for improved maintainability for the entire solution.

Filed under:

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Comments

# TEK said on March 5, 2007 9:26 AM:

I look at the code examples, and what strikes me is how this would work in a real-life situation.

In my experience many programmers have more than enough with finding out how to do stuff using the .Net Framework classes directly.

Introducing this kind of abstraction througout the code of a large app seems like a daunting task when there are several people that is going to write the code.

Maybe is the solution to this to have a good documentation about how to do stuff, but do to this in a enteprise app would cause anyone working on the project to have to learn a lot of new syntax. And, more imprtantly, remember to write that syntax themself.

# andersnoras said on March 5, 2007 1:03 PM:

TEK,

I can sympathize with your view on this. I've often seen developers struggle to learn the base class libraries of the platform they are using. The best solution is to mentor developers on intermediate level concepts like inversion of control. If this is not an option, it might be a good idea to encapsulate the IoC resolving logic in a class with a more familiar interface. With Oren's example you could for instance do this:

public static class File

{

   public static void WriteAllText(string path, string text)

   {

       using(TextWriter writer =

            IoC.Resolve<IFileWriterFactory>().

                  Create(filename))

       writer.Write(text);    

   }

}

In this way you can keep control of the implementation, achieve testability and retain readability of the code. The trade-off this time is that you introduce another level of abstraction that is not needed and that you'll write more code than what is actually needed.

# Eli Lopian said on March 6, 2007 12:02 AM:

Anders,

Great discussion.

"We should be careful about mixing up maintainability and readability."

I think that we should be more careful not to mix up maintainability and extendability.

Having an extend-able system is the old school (waterfall) way of having a Maintainable system, but we now know that the price of doing this actually makes maintenance harder. So the Agile way was created:

In the Agile way we make the system as Simple as Possible, this make it maintainable and we have a Just In Time Design phase where we can change our design easily to add more features. (Supported by tests and re-factoring tools). The extendability comes from the process and not the design!!!

Testability is non issue once you have the correct tools (TypeMock). So all your code is always Testable - no brainer here.

Orens, complex design was only to make the code testable, he then found another use for it. This is a classic YAGNI, old school - "See what a great design I made it solved the <whatever> problem that came up months after the design", what about all the wasted hours in creating extend-able code that is never really used.

# Eli Lopian’s Blog (TypeMock) » Blog Archive » IoC and Average Programmers said on March 6, 2007 1:01 PM:

PingBack from http://www.elilopian.com/2007/03/05/ioc-and-average-programmers/

# Anders Norås' Blog said on March 6, 2007 1:06 PM:

In the wake of the big discussion on designing for testability and average developers , I stumbled across

# TEK said on March 6, 2007 5:43 PM:

Anders,

Have to agree with Eli on this.

I think you just solved one problem by adding a different one.

Now we have two classes, System.IO.File and MyStuff.IO.File.

And they both do exactly the same...

Except that the the MyStuff.IO.File problably is going to be missing "the other method" I need.

This is what that other developer is going to be thinking:

"And as I'm a rookie, or just in a really-really big hurry, I'm just going to use that System.IO.File for this small feature. And I'll come back and fix it later."

Maybe it will break the test, maybe not?

Anyway, maybe I'm a bit pragmatic here, but in my mind what is done is to introduce a number of additonal code into your production code to add testing support.

For the purpose of testing, you have also added a new possible failing point into your system.

In my mind the IoC.Resolve<IFileWriterFactory>() is a possible failing point during run time.

And how will the test adapter react to other stuff, for example illegal signs like :/? in the filename?

The behaviour in those cases should also be an interesting aspect of your test code. But now your result may report the wrong answare, as your test adapter does not report the correct errors if the filename has invalid chars.

Other examples is how does the adapter handle long paths? or long filenames? Or existing files? Or missing directory?

To me it kindof seems like you are adding the possiblity to easy write test code but the test is not testing anything near your real life situation.

How much good will the test do you then?

I know this is a bit extreme, but there core of the problem is absolutly valid!

# andersnoras said on March 7, 2007 1:44 AM:

TEK,

I agree that creating a separate class to hide the dependency resolving code has more cons than pros. The only pro is that a novice developer who does not grasp IoC-concepts are shielded from the IoC code.

Even if I would not recommend that you resolve every dependency in an application through IoC, I believe it is an excellent tool to loosen up dependencies when the need for this arises.

The file writer example taken from Oren's blog on seams (http://ayende.com/Blog/archive/2007/03/03/The-Production-Value-of-Seams.aspx).   Oren's application created files that would be transferred to a mainframe system. However, if the file wasn't complete the mainframe import process would fail. Oren solved this by creating two files rather than one.

Another example would be if you needed to create a file with EBCDIC encoding or similar that isn't inherently supported by .NET alongside more common encodings. This could be configured and handled by the injected service without effecting the client code.

As for your concerns on illegal characters, path lengths and similar I would agree that this would be a serious issue if you were to write your own IO-handling, but this is not the case here. The IFileWriterFactory is simply a wrapper around the types found in the System.IO namespace so any exceptions thrown by these classes will be propagated to the caller.

# TEK said on March 7, 2007 11:21 AM:

Thanks for your feedback.

I'm just a bit sceptical about anything that is different during testing and real-life, as they (if done wrong) have a bad tendency to pop-up after you have released the program. Often due to configuration issues or something like that.

I'll want getting into a discussion based on the actual usability for the IoC framework, for that I know way to little about how it works.

Good discussion, thanks!

Regards, Trond-Eirik

# GutZofter said on July 20, 2007 6:29 AM:

Maybe this would be a little better (FI):

File.Write(text).To(filename);

# Alec Lazarescu's Blog said on February 18, 2008 2:40 PM:

Inversion of Control (IoC) is often achieved by Dependency Injection and programming to interfaces and

Leave a Comment

(required) 
(optional)
(required) 
Enter the code you see below