Navigate / search

Aim for DRY, but be willing to fall short.

I’m not going to rehash the reasons why “Don’t repeat yourself” (DRY) is a good thing, I’ll assume you know why duplicating code is bad.

Strictly speaking DRY isn’t really about repeating code, it’s more about representing the same “Information” or “Knowledge” in different places. For that to happen the code doesn’t actually have to be the same.

Conversely, duplicated code doesn’t always represent duplicated knowledge or information, completely different concepts can sometimes be represented by virtually identical code.

Mathias Verraes recently wrote a really good blog post on this very issue, it’s well worth a look.

This post is about aiming for DRY, but deliberately coming up short.

I’ve been adding some NUnit tests to an existing system recently and I kind of liked the way they turned out, despite the fact that I duplicated some code.

The following is heavily modified code, I can’t show the original, but it gets the point across. Please don’t read into anything you see about credentials or security, this code is one step away from Widgets and Wotzits, it’s NOT REAL CODE.

    class Describe_Club_Membership
    {
        [Test]
        public void A_User_Can_Join_The_Club()
        {
            UsingCredentials("appName", "appKey");
            var userId = GetUserId("someUsersEmail@example.com");

            EnsureUserIsNotAMember(userId)

            JoinClub(userId);
            Assert.That(IsAMember(userId));
        }

        [Test]
        public void A_User_Can_Leave_The_Club()
        {
            UsingCredentials("appName", "appKey");
            var userId = GetUserId("someUsersEmail@example.com");

            EnsureUserIsAMember(userId)

            LeaveClub(userId);
            Assert.That(IsNotAMember(userId));
        }

The entire contents of the tests are written using functions that exist only for the purpose of making the tests readable. In a sense these few functions define a Domain Specific Language. Code within any function (but particularly within a test) should always be written at the same level of Abstraction, would should never have code like the following in a test.

    class Describe_Club_Membership
    {
        [Test]
        public void A_User_Can_Join_The_Club()
        {
            var credentialsFactory = new CredentialsFactory("Membership");
            var credentials = credentialsFactory.CreateDefaultCredentials("appName", "appKey");

            var registrationRepo = new RegistrationsRepository(credentials);
            var userId = registrationRepo.GetUserByEmail("someUsersEmail@example.com").FirstOrDefault().Id;

            var binding = new BasicHttpBinding();
            var membershipEndPoint = new EndpointAddress(ConfigurationManager.AppSettings["..."]);
            
            var membershipService = new MembershipServiceClient(binding, membershipEndPoint);

            ... etc...   
        }

You get the idea, everything that needs to be done, is in the test, from pulling information out of Repositories, to reading Configuration Files, to configuring services endpoints to actually calling services, and on and on.

I don’t know about you, but I prefer the first code sample to the second.

Those functions that make up the Domain Specific Language in the first tests, have to go somewhere, and as you can see from the second example, some of them involve a little bit of work. Look again at the first code sample, you’ll see that I have a function called ‘UsingCredentials’ but it doesn’t return credentials. If it did I could pass them into the subsequent functions that need credentials, but in my DSL it’s enough to state the credentials I want to use. Passing the credentials and all the other dependencies around would unnecessarily clutter up the tests.

So, clearly the credentials are being saved somewhere by the ‘UsingCredentials’ function and then subsequent functions can use them.

    class Describe_Club_Membership
    {
        private Credentials _credentials;

        [Test]
        public void A_User_Can_Join_The_Club()
        {
            UsingCredentials("appName", "appKey");
            var userId = GetUserId("someUsersEmail@example.com");

            EnsureUserIsNotAMember(userId)

            JoinClub(userId);
            Assert.That(IsAMember(userId));
        }

        // Test Fixture DSL Commands
        private void UsingCredentials(string appName, string appKey)
        {
            _credentials = CredentialsHelper.GetCredentials(appName, appKey);            
        }

        ...

The UsingCredentials function is actually just a one liner, the heavy lifting is done elsewhere in a helper class. The job of getting credentials may be needed by lots of TestFixtures so I want that logic in a shared component, not duplicated. We’re heading in the right direction of DRY.

So, what then is the point of the UsingCredentials function we see here?

Put simply, I like the way it makes the tests read. I could have used this one liner, and others like it in the test, but a test made up of a series of these kinds of calls is more cluttered and it gives the impression that these helper classes may be significant to the test. I don’t want to give that impression.

I want to declutter the test so that only the logic being tested remains. Wrapping this and other one liners in tiny DSL style commands sends a message that the details of how the helpers are called is not as important than the logic being tested.

But what then if I want to use the same Helper from other Test Fixtures, aren’t I going to have to duplicate the little functions that wrap the one liners?

Yes I am.

This is where I deliberately fall short of DRY.

For one of my fixtures I have 7 of these tiny functions. They vary in complexity but none contain more than one line of code. Code that I do not want to see directly in my tests. In another fixture, I use three of those same wrapper functions, and I’ve duplicated the three that I need.

How could I make use of these DSL functions without duplicating them? One benefit of the wrapper functions is that they can access the credentials or any other member of the fixture without it being passed as a dependency. If I move the wrapper functions out I’m basically back to calling the helper functions and needing to pass all dependencies. In short, the wrapper functions no longer serve any purpose.

I could define the helpers and the credentials in a base class and inherit my test fixtures from that. That is in fact a really nice solution, it’s DRY, the Tests look just as nice and the Test Fixtures don’t even have the tiny wrapper functions in them.

If you wanted to go with the inheritance option I’d be hard pushed to make a case against it. I just don’t like inheritance and I really don’t like it in test suites.

Irrational as it may be I prefer to have these tiny functions so my tests can be as clean as possible, but I’m willing to fall a little short of DRY rather than use inheritance.

If someone is curious when looking at a test and wants to see what a function does or where credentials come from, it’s right there, in the fixture, no climbing an inheritance hierarchy.

As soon as you start using inheritance it sucks you in, you start adding more to the base class, or an extra level of inheritance here and there. It gets messy real fast.

What will happen when not all of the test Fixtures need Credentials? When some subset of fixtures need some other shared member? The Base class starts to become the union of all needs rather than the intersection. And for what? So we can avoid duplicating a handful of one line functions that are unlikely to change. Even if they do change it’s going to be trivial, perhaps a new argument to the helper function.

The key points for me are:

  1. Keep your tests as free of clutter as possible, nothing should remain but the logic being tested, written at a consistent level of abstraction.
  2. If necessary define a tiny Domain Specific Language for your tests as simple one liner functions which call to the real shared code, kept elsewhere.
  3. Keep the DSL functions you need right there in the Test Fixture, duplicate them if necessary. Don’t try to over engineer your DSL using inheritance or any other clever tricks.

The third point about inheritance is maybe up for debate, but I don’t thing the first two points are.

Leave a comment

name*

email* (not published)

website