…because I almost never agree with him!

For instance, in this post – inspired by the book, Coders At Work – he describes the character he calls “the duct tape programmer”, who is the archetype of that annoyingly great programmer, who just seems to be able to solve any kind of problem with only a few tools.

One of the attributes of “the duct tape programmer” is that he seldomly wastes time writing tests, because “(…) the customer isn’t going to complain about that”. Another is that he does not succumb to hype and trends in software development, he is confident that what he knows is enough.

And so Joel goes, on and on about this fantastic programmer, who’s capable of delivering what the customer wants without all the fancy hypes brand-spanking-new technologies and methodologies that all the other programmers seem to waste their time with.

The problems, as I see it, are: a) Very few programmers are like that (you know, that Linus Torvals/Bjarne Stroustrup kind of guy), and b) I will NEVER be the guy who maintains his code!

The thing is – to continue Joel’s analogy – that “the duct tape programmer” might be able to win a go-cart race with a go-cart made of duct tape and WD-40, but I would NOT trust him to make a car for me to drive on the highways! – because I would not expect that car to be safe at high speed, and I would not expect it to last for years, and I would not expect it to withstand rain, snow, sand, etc.

In my opinion, automated unit tests and integration tests serve many purposes. One (obvious) one is to continually verify the behavior of a system, given that implementations may change and bugs be fixed and so on. Another is to DOCUMENT the behavior of the system, like in “we agreed with the customer that it should work like this”. Yet another is to aid the programmer while writing his code into separating concerns, because doing that just causes less pain when writing tests. Joel should stop bashing TDD.

And when it comes to new technologies and methodologies, I think it is SO IMPORTANT to keep an eye open for new AND old better ways to do stuff. My opinion is that I get smarter all the time by keeping an open eye on what other people are thinking and doing, and I get smarter all the time by putting some of it to use. Joel is just annoying here, because his statements lack nuance – keeping up with new stuff is NOT the same as bringing in new stuff unconsciously, but Joel does not seem to be capable of drawing this distinction.

In my opinion, there is no place for “the duct tape programmer” on a team. At least not on my team.

So if you care about your teammates, about the quality AS WELL AS THE MAINTAINABILITY AND EXPLAINABILITY of your code, please DO write tests!

And please DO separate concerns and take the time to factor out stuff into easy-to-understand narrow and focused classes!

And please, please DO use new as well as old technologies and practices where it makes sense – don’t put stuff off because it’s new, and you are used to hand-rolling your own linked lists and string structs in C and the C++ STL is just another fancy new hype to you.

There’s one thing, that almost always makes we want to assume the foetal position and cry: developers, who are ignorant to the fact that there is a difference between application logic and application framework.

I must admit that I have only recently started being (this) conscious about this difference, so I have written a buttload of code the last few years that violates almost everything that I stand for now, which really makes me sad inside. But it’s never too late to improve.

Once I realized this and started to try to adhere to it to keep the two things separated, I started seeing things so very clearly, and then other people’s ignorance of this fact just started to gnaw and irritate me. Hence this post – I need to get this off my chest – I need to write another post in the “rant” category…

An incredibly insightful (as always) post was made by Ayende a couple of months ago: Application structure: Concepts & features. Ayende’s post explains it so well, but basically he distinguished between concepts in a system, which is stuff that requires design, and features which merely build and/or augment existing concepts. I just want to add a personal experience to the rant in the context of Ayende’s post.

Here’s the setting

I am currently on a team that develops and maintains a mortgage deed trading and administration system. Part of this system is an extensive suite of automated nightly jobs and automatic reports.

Naturally, some of the reports are run every night, e.g. summing up the numbers recorded the previous day for automated export to accounting systems, other reports are run every week/month/year, some on bank days, some relative to bank days, etc.

Some jobs make changes to the state of the system (lige e.g. updating the particulars of people for whom we receive updates from the Central Office of Civil Registration, remembering that a particular batch of transactions have been exported to the accounting system, or remembering that information on interest fees for the previous year have been reported to the National Bank etc.), and some are just (idempotent) reports and exports.

Most jobs run automatically in the night. Some jobs can also be initiated by the user of our Windows Forms frontend through a web service call. And all reports should also be accessible through the built-in reporting frontend in the Winforms app.

Here’s our current solution

All reports are run through a web service call by instantiating a ReportCommand, which is capable of getting the names of all the reports accessible to the current user. And then, given a report name, the command can get all the parameters for that report. And then, given a report name and a set of parameters, it can run the report and output a file in SpreadSheetML format. This allows our frontend to dynamically build a GUI for all reports. No GUI work is needed whenever we need to create a new report, which is great.

The majority of our nightjobs are initiated by the Windows Scheduler, which stems from an old pragmatic solution to the very first automated job we needed almost three years ago. This has not changed, so jobs are still scheduled manually through the Windows Scheduler. Our job runner is an ordinary .NET Windows .exe, which gets executed with one or more arguments. Exporting transactions to accounting could look like this:

jobrunner.exe export_transactions

- which would use the string to look up a class that implements ITask, e.g. ExportTransactionsTask, which exports all non-exported transactions in a .csv file to some preconfigured location.

To be able to schedule reports to run, we have made a job named “report”, which is capable of invoking the ReportCommand directly, setting the parameters of the report from the given command line arguments. Running a report could look like this:

jobrunner.exe report -name transactions -caseNo 10000:20000 -recordDate today-1:today -mail ceo@capitalism.com

- which runs the ReportTask, which is fed a dictionary containing {{"name", "transactions"}, {"caseNo", "10000:20000"}, {"recordDate", "today-1:today"}, {"mail", "ceo@capitalism.com"}}, which in turn runs the ReportCommand with the given report name and parameters, extrapolating macros like “today” into today’s date + some more simple stuff.

If this sounds complicated to you, then yes! It is actually pretty complicated! Not because it should be, but because it’s pretty legacy and implemented in a pretty messy way. And now the need has come for the users to be able to schedule jobs and reports from the Winforms frontend. Damn! This is a good time to reflect on how I wish we had done it (and how I plan to do stuff like this in the future).

How I wish we had done it

The problem above, as I see it, is that features and concepts are mixed together in a gray matter that noone can oversee. I wish we had thought more about separating the concepts (command, nightjob, report) from the features (implementation of the different jobs and reports). I wish implementing a new report was easy like this (#region...#endregion added as explanation):

public class TransactionsReport : Report, IHasParameters, IGeneratesFileResults, IWantsToSendEmails
{
    #region Report members
    public override void Run()
    {
        // reporting logic in here
        // ... querying the db for stuff like Parameters.RecordDates.From 
        // etc...
    }
    #endregion
 
    #region IGeneratesFileResults members
    public override IEnumerable<FileResult> GetFileResults()
    {
        // return information about generated files in here
    }
    #endregion
 
    #region IWantsToSendEmails members
    public override IEnumerable<EmailResult> GetEmailResults()
    {
        // return information about emails to be sent here
    }
    #endregion
 
    // IHasParameters is a marker interface, that makes our framework look for the "Parameters"
    // property and reflect over its type
    public override Parameters Parameters { set; get; }
 
    public class Parameters
    {
        [Parameter("Record dates", Description="The report contains transactions recorded withing the given date range.")]
        public DateRange RecordDates { get; set; }
 
        [Parameters("Deed stores", Description="Transactions recorded in store with number within the given range.")]
        public IntegerRange StoreNumbers { get; set; }
    }
}

and then all possible implementations of the Report class would be picked up by the various concepts in the system, like e.g. our report command, the report scheduler, and an ad hoc reporting task runner… and then, in a similar fashion, I want to subclass an IdempotentTask class for all tasks, that do not change anything in the system, and a TaskWithSideEffects class for all tasks that change the state of the world.

This way, implementing the logic inside of reports and tasks will be orthogonal to implementing the capabilities of the reports and tasks and their scheduling.

As a response to Ben Scheirman’s post, Benjamin Day kindly apologized and summed up why he likes Visual Studio Team System and Team Foundation Server.

I am not going into the debate on whether it was right or wrong to delete that comment, because a lot of people already did that, and I agree with those who think that deleting the comment was kind of wrong. Calling it “unethical behaviour“, however, seems to be a little too harsh. Moderating news channels discussing politics in China is unethical – deleting a comment, because the blog author disagrees, is just weird and a little bit annoying.

Instead, I just wanted to chime in with my 2 cents on why I think Visual Studio Team System and Team Foundation Server are inferior, compared to ALL of the free alternatives that I know of – it’s because I believe in one of the finest principles of software engineering, which was coined by Edsger Dijkstra: Separation Of Concerns.

Separation Of Concerns can be low level, as in Uncle Bob’s single responsibility principle, or higher level as in service-orientation, or even higher level as in there’s NO WAY I’m gonna buy an oven, which insists on also being my washing machine and a pair of roller blades. No way!

This principle is so inherent in all the good disciplines of software engineering, heck in LIFE even, that I simply had to reply!

So I like to use CruiseControl, TeamCity, Subversion, Git, ReSharper, TestDriven, NUnit, xUnit.net, Jira, Redmine, Basecamp, MSBuild, Rake, NAnt etc. etc. because they let me switch each one of them out for anyone of the other whenever I feel like it. And, more importantly, whenever it fits the task better.

The fact that some of the tools are FREE and have their source code available for me to look at, is just an added plus. But the primary reason to use those tools is simply that they do one thing, and they are usually capable of doing that one thing better.

When two things are orthogonal, it means that the angle between them is 90 degrees – at least in spaces with 3 dimensions or less. So when two vectors are orthogonal, they satisfy the property that there is no way to use the first one to express even the tiniest bit of what the other one expresses.

That is also how we should write our application code: methods and classes should be orthogonal to one another – i.e. no class should try to express what another class already expresses either in part or whole – therefore each class and each method should have only one responsibility, and thus one reason to change.

And test code is real code.

The corollary is that our tests should have only one single reponsibility as well.

That is why I hate tests that look like this:

public void CanRecordPayment()
{
    CreateMortgageDeed(1, new DateTime(2003, 3, 11));
    CreateMortgageDeed(2, new DateTime(2003, 6, 11));
    CreateMortgageDeed(3, new DateTime(2003, 3, 11));
    CreateMortgageDeed(4, new DateTime(2003, 3, 11));
    CreateMortgageDeed(5, new DateTime(2003, 9, 11));
 
    var mortgageDeeds = new DueTermsFinder(new MortgageDeedRepository())
        .FindDueTermsBefore(new DateTime(2003, 4, 1));
 
    Assert.AreEqual(3, mortgageDeeds.Count);
 
    mortgageDeeds = mortgageDeeds.OrderBy(m => m.CaseNo);
 
    Assert.AreEqual(1, mortgageDeeds[0].CaseNo);
    Assert.AreEqual(3, mortgageDeeds[1].CaseNo);
    Assert.AreEqual(4, mortgageDeeds[2].CaseNo);
 
    var result = new TermDebitRecorder()
        .CreateAndRecordTermDebits(mortgageDeeds);
 
    Assert.AreEqual(3, result.RecordedTermDebits.Count);
 
    Assert.AreEqual(new DateTime(2003, 3, 11), result.RecordedTermDebits[0].TermDate);
    Assert.AreEqual(new DateTime(2003, 3, 11), result.RecordedTermDebits[1].TermDate);
    Assert.AreEqual(new DateTime(2003, 3, 11), result.RecordedTermDebits[2].TermDate);
    // .... etc!
}

Notice how this test is actually fairly decently structured – at least that’s what it initiallt looks like… but it actually tests a lot of things: it checks that the output of the DueTermsFinder is what it expects, testing the MortgageDeedRepository indirectly as well – and then it goes on to test the TermDebitRecorder … sigh!

If (when!) one of these classes changes at some point, because the requirements have changed or whatever, the test will break for no good reason. The test should break because you have introduced a bug, not because you made a change in some related functionality.

That is why I usually follow the pattern of AAA: Arrange, Act, Assert. Each test should be divided into discrete steps corresponding to 1) Arranging some data, 2) Triggering a computation or some state change, 3) Asserting that the outcome was what we expected. And if I am feeling idealistic that day, I also follow the principle of putting only one assertion at the end of each test.

I try to never do AAAAA (Arrance, Act, Assert, Act Assert) or AAAAAA, or AAAAAAA which is even worse.

Every test should have only one reason to break.

When writing code, I often end up introducing a layer supertype – i.e. a base class with functionality shared by all implementations in that particular layer in my application.

This also holds for my test code – and why shouldn’t it? Test code is as real as real code, so the same rules apply and it should benefit from the same pain killers as we implement in our application code.

For example when testing repositories and services that need to query the database, I can save myself a lot of writing by stuffing all the boring NHibernate push-ups in a DbTestFixture supertype – this includes building a configuration that connects to a test database, building a session factory, storing that session factory somewhere, re-creating the entire database schema in the test fixture setup, and running each test in a transaction that is automatically rolled back at the end of each test + a few convenience methods that allow me to flush the current session etc.

The DbTestFixture might look something like this (note that all my repositories take an instance of ISessionProvider in their ctor – that’s how they obtain the currently ongoing session, which is why I have a TestSessionProvider to inject into repositories under test):

public class DbTestFixture
{
	TestSessionProvider currentSessionProvider;
	ISession currentSession;
	ITransaction currentTransaction;
	bool commit;
 
	[TestFixtureSetUp]
	public void TestFixtureSetUp()
	{
		SessionFactoryHolder.Instance.CreateSchema();
 
		using (currentSession = OpenSession())
		{
			DoTestFixtureSetUp();
			currentSession.Flush();
		}
	}
 
	[SetUp]
	public void SetUp()
	{
		currentSession = OpenSession();
		currentTransaction = currentSession.BeginTransaction();
		currentSessionProvider = new TestSessionProvider(currentSession);
 
		DoSetUp();
 
		currentSession.Flush();
	}
 
	[TearDown]
	public void TearDown()
	{
		DoTearDown();
 
		if (commit)
		{
			currentTransaction.Commit();
		}
		else
		{
			currentTransaction.Rollback();
		}
 
		currentTransaction.Dispose();
		currentSession.Dispose();
	}
 
	protected ISession OpenSession()
	{
		return SessionFactoryHolder.Instance.SessionFactory.OpenSession();
	}
 
	protected ISessionProvider SessionProvider
	{
		get { return currentSessionProvider; }
	}
 
	protected virtual void DoTestFixtureSetUp() { }
	protected virtual void DoSetUp() { }
	protected virtual void DoTearDown() { }
 
	protected ISession CurrentSession
	{
		get { return currentSession; }
	}
 
	protected T Reload<T>(T t) where T : Base
	{
		SaveAndFlushAndClear(t);
		return CurrentSession.Get<T>(t.Id);
	}
 
	protected void SaveAndFlushAndClear(object obj)
	{
		CurrentSession.Save(obj);
		CurrentSession.Flush();
		CurrentSession.Clear();
	}
 
	protected void SaveAndFlush(object obj)
	{
		CurrentSession.Save(obj);
		CurrentSession.Flush();
	}
 
	protected void Save(object obj)
	{
		CurrentSession.Save(obj);
	}
 
	protected void Flush()
	{
		CurrentSession.Flush();
	}
 
	protected void DoNotRollBack()
	{
		commit = true;
	}
 
	protected class TestSessionProvider : ISessionProvider
	{
		readonly ISession currentSession;
 
		public TestSessionProvider(ISession currentSession)
		{
			this.currentSession = currentSession;
		}
 
		public ISession GetCurrentSession()
		{
			return currentSession;
		}
 
		public void Commit()
		{
			throw new System.NotImplementedException();
		}
 
		public void RollBack()
		{
			throw new System.NotImplementedException();
		}
	}
}

Then a fictional repository test might look as simple as this:

[TestFixture]
public class TestMessageRepository : DbTestFixture
{
	MessageRepository repo;
	Guid messageId;
 
	protected override void DoTestFixtureSetUp()
	{
		var message = new Message
		              	{
		              		CreatedAt = new DateTime(2003, 11, 11),
		              		Subject = "some subject",
		              		Body = "some body",
		              	};
 
		CurrentSession.Save(message);
 
		messageId = message.Id;
	}
 
	protected override void DoSetUp()
	{
		repo = new MessageRepository(SessionProvider);
	}
 
	[Test]
	public void CanLoadSingleMessage()
	{
		var message = repo.Find(messageId);
 
		Assert.AreEqual(new DateTime(2003, 11, 11), message.CreatedAt);
		Assert.AreEqual("some subject", message.Subject);
		Assert.AreEqual("some body", message.Body);
	}
}

Note how DbTestFixture flushes in all the right places so I don’t need to worry about that.

This test fixture supertype can be used for all my database access tests, as well as integration testing. But what about unit tests? I am using Rhino Mocks, so my unit test fixture base looks like this:

public class UnitTestFixture
{
	protected readonly MockRepository mocks = new MockRepository();
 
	protected T Stub<T>(params object[] paramsForConstructor) where T : class
	{
		return mocks.Stub<T>(paramsForConstructor);
	}
 
	protected T Mock<T>(params object[] paramsForConstructor) where T : class
	{
		return mocks.DynamicMock<T>(paramsForConstructor);
	}
}

Real simple – it just stores my MockRepository and gives me a few shortcuts to the mocks I care for. Then I inherit this further to ease testing e.g. my ASP.NET MVC controllers like this:

public abstract class ControllerTestFixture<T> : UnitTestFixture where T : Controller
{
	protected T controller;
 
	[SetUp]
	public void SetUp()
	{
		controller = CreateController();
	}
 
	protected abstract T CreateController();
}

As you can see, I make it a real “fixture” – the controllers I am about to test will fit into this fixture like a glove, and I will certainly never forget to instantiate my controller only once, because I start out by implemeting that part in the implementation of the CreateController method.

A controller test might look like this:

 
[TestFixture]
public class TestAuthenticationController : ControllerTestFixture<AuthenticationController>
{
	ISessionContext sessionContext;
	IAuthenticationService authenticationService;
 
	protected override AuthenticationController CreateController()
	{
		sessionContext = mocks.DynamicMock<ISessionContext>();
		authenticationService = mocks.DynamicMock<IAuthenticationService>();
 
		return new AuthenticationController(sessionContext, authenticationService);
	}
 
	// tests down here...
}

One thing, that I find really annoying, is that somehow it seems to be accepted that test code creates instances of this and that like crazy! In a system I am currently maintaining with about 1MLOC where ~40% is test code, I often come across test fixtures with something like 20 individual tests, and each and every one of them creates instances of maybe 5 different entities, builds an object graph, runs some code to be tested, and does some assertions on the result.

This is a super example on how to write brittle and rigid tests, because what happens if the signature of one of the ctors changes? Or the semantics of the object graph? Or [insert way too many ways for the test to break for the wrong reasons here...].

When I come across these tests, I usually factor out the creation of all but the most simple objects behind methods with meaningful names. This has two advantages: 1) It’s more DRY because they can be re-used, and 2) It serves as brilliant documentation. Consider this rather simple assertion that requires a few objects to be in place:

[Test]
public void MortgageDeedKnowsItsActualPrincipalBeforeAmortizationHasBegun()
{
    var deed = new MortgageDeed();
 
    deed.Principal = 100000;
    deed.Amortization.FirstTerm = new DateTime(2003, 3, 11);
 
    deed.Amortization.AddAmortizationStrategyElement(new AnnuityAmortizationStrategyElement
    {
        FirstTerm = 1,
        TermsPerYear = 4,
        PeriodicPayment = 5000,
    });
 
    deed.Amortization.AddInterestStrategyElement(new FixedRateInterestStrategyElement
    {
        FirstTerm = 1,
        Rate = 0.05,
    });
 
    Assert.AreEqual(new DateTime(2003, 3, 11), deed.ActualPrincipalDate);
}

compared to this:

[Test]
public void MortgageDeedKnowsItsActualPrincipalDateBeforeAmortizationHasBegun()
{
    var deed = NewStandardMortgageDeedBeforeAmortization(new DateTime(2003, 3, 11), 100000, 5000);
 
    Assert.AreEqual(new DateTime(2003, 3, 11), deed.ActualPrincipalDate);
}
 
MortgageDeed NewStandardMortgageDeedBeforeAmortization(DateTime firstTerm, decimal principal, decimal periodicPayment)
{
    //...
}

MUCH more clear! The factory method acts as brilliant documentation on which aspects of the test are relevant to the outcome – it is clear, that a mortgage deed which has not yet begun its amortization must report its first term date as the actual principal date.

Go on to test another property of the mortgage deed before amortization:

[Test]
public void MortgageDeedKnowsItsActualPrincipalBeforeAmortizationHasBegun()
{
    var deed = NewStandardMortgageDeedBeforeAmortization(new DateTime(2003, 3, 11), 100000);
 
    Assert.AreEqual(100000, deed.ActualPrincipal);
}

- and we have already saved us from writing 50 lines of brittle rigid test code. Keep factoring out common stuff, so that the ctor of the mortage deed is still only called in one place… e.g. create methods like this:

MortgageDeed NewStandardMortgageDeedWithTransactions(
    DateTime firstTerm, 
    decimal principal, 
    decimal periodicPayment, 
    params Transaction [] transactions)
{
    var deed = NewStandardMortgageDeedBeforeAmortization(firstTerm, principal, periodicPayment);
    Array.ForEach(transactions, t => deed.AddTransaction(t));
}
 
Transaction NewPayment(DateTime paymentDate, decimal amount)
{
    var payment new PaymentTransaction
    {
        Amount = amount,
        PaymentDate = paymentDate,
        ValueDate = paymentDate,
    };
 
    payment.Record();
 
    return payment;
}
 
Transaction NewTermDebit(DateTime termDate, decimal interest, decimal installment)
{
    var termDebit = new TermDebitTransaction
    {
        Interest = interest,
        Installment = installment,
        TermDate = termDate,
    }
 
    termDebit.Record();
 
    return termDebit;
}

which allows me to write cute easy-to-understand tests like this:

[Test]
public void ActualPrincipalDateIsChangedWhenPaymentIsMade()
{
    var deed = NewStandardMortgageDeedWithTransactions(
        new DateTime(2003, 3, 11), 
        100000,
        NewPayment(new DateTime(2003, 6, 11), 5000));
 
    Assert.AreEqual(new DateTime(2003, 6, 11), deed.ActualPrincipalDate);
    Assert.AreEqual(new DateTime(2003, 3, 11), deed.AmortizedPrincipalDate);
}
 
[Test]
public void AmortizedPrincipalDateIsChangedWhenTermDebitIsMade()
{
    var deed = NewStandardMortgageDeedWithTransactions(
        new DateTime(2003, 3, 11), 
        100000,
        NewTermDebit(new DateTime(2003, 6, 11), 4000, 1000));
 
    Assert.AreEqual(new DateTime(2003, 3, 11), deed.ActualPrincipalDate);
    Assert.AreEqual(new DateTime(2003, 6, 11), deed.AmortizedPrincipalDate);
}

This is also a good example on how to avoid writing // bla bla comments all over the place – it’s just not necessary when the methods have sufficiently well-describing names.

May 292009

Sometimes I think it is funny to reminisce a bit about where I was and where I am now. One area where I think I have moved quite a lot, is in regards to testing! And I can’t help to think that part of this “journey” was necessary to learn what I think I have learned, but somehow I still can’t put off the thought that I might have gotten here quicker if someone had told me some basic stuff… therefore, I will try to capture in this post a few points, that I pay attention to when writing tests – in hopes that someone might benefit from a little guidance, and hoping not to be categorized as an old man rambling on about his own percieved experience.

To make it more digestible, I will list some points and show examples on why each point is good where I see fit.

  1. Hide object instantiation behind methods with meaningful names
  2. Create base classes for your test fixtures
  3. Make your tests orthogonal
© 2010 mookid on code Suffusion WordPress theme by Sayontan Sinha