TDD and Single Responsibility Principle violation - a simple example

As many have already stated, TDD encourages simple, elegant design. Today I'd like to show you a simple example on how you can smell Single Responsibility Principle violation from your specs (AKA unit tests).

A session storage

Imagine we're building a web server which allows different clients connect to it via HTTP requests. The server is so simple that it holds all its sessions for different clients in memory. One of the specs that led to implementing this in-memory session storage looks like this:

[Test] public void
ShouldKillAllContainedSessionsWhenItIsKilled()
{
  var sessions = new Sessions();
  var session1 = Substitute.For<Session>();
  var session2 = Substitute.For<Session>();

  sessions.Add(session1);
  sessions.Add(session2);

  sessions.Kill();

  session1.Received().Kill();
  session2.Received().Kill();
}

Now imagine that a company became interested in our web server and wants to turn it into a commercial product. The company strategy is as follows: users will be offered free version to try things out and when they pay, they will receive activation code.

There is only one difference between the trial and full version - the trial version has a limit of only one active session at a given moment.

This change of policy needs to be reflected in the session storage. So we add a logic that checks for the session limit and throws an exception as soon as the limit is exceeded. A specification of such behavior would look like this:

[Test] public void
ShouldThrowExceptionWhenTryingToAddSessionsBeyondDefaultLimitOfOne()
{
  var sessions = new Sessions();
  var session1 = Any.InstanceOf<Session>();
  var session2 = Any.InstanceOf<Session>();

  sessions.Add(session1);
  
  Assert.Throws<Exception>(() => sessions.Add(session2));
}

This, however, leaves us with another issue: what about the first spec mentioned in this post? If the limit of sessions is 1 by default, it will fail, since we're adding two sessions there. So we'll need to somehow extend the limit for the purpose of this spec.

I assume that you're smart enough not to use Setup and Teardown methods in your unit-level specs (I'll have a post about why Setup and Teardown usually stink someday), so your "corrected" spec looks like this now:

[Test] public void
ShouldKillAllContainedSessionsWhenItIsKilled()
{
  var sessions = new Sessions();
  var session1 = Substitute.For<Session>();
  var session2 = Substitute.For<Session>();
  sessions.Limit = 2;

  sessions.Add(session1);
  sessions.Add(session2);

  sessions.Kill();

  session1.Received().Kill();
  session2.Received().Kill();
}

And so we added extending the session limit to 2. Now, doesn't this bother you? Look at the name of the spec - does it tell anything about a session limit? No, it doesn't. Let's think about the behavior as well - did it become invalid after we introduced the session limit? No, it didn't.

So how did it happen that the session limit concept leaked to an unrelated specification? Is this a problem with the spec?

No, it isn't. It's a problem with our production code design. In fact, this spec is just a symptom of what we did with the design:

We violated Single Responsibility Principle

That's right, folks, we really did it. Currently, the session storage is responsible not only for storing and accessing the sessions, it also knows EXACTLY what is the business-imposed limit and takes care of enforcing this limit. That's two responsibilities, not one.

Ok, so we screwed up. How do we fix it?

The easiest way is to have the limit-related logic isolated in a separate class. Because we want this class to be a result of implementing specifications (remember, test-driven ;-)), we undo the addition of the limit-related code to session storage class (and existing unit tests) and start over. Here's an example spec that will drive an implementation that respects Single Responsibility Principle. Implementing it will not impact other specs any way other than with a need to supply a dummy constructor parameter.

[Test] public void
ShouldValidateNumberOfSessionsBeforeAddingANewOne()
{
  var sessionCountValidation 
    = Substitute.For<SessionCountValidation>()
  var sessions = new Sessions(sessionCountValidation);
  var sessionsCountBeforeAddingNewOne = sessions.Count;

  sessions.Add(Any.InstanceOf<Session>());

  sessionCountValidation.Received()
    .Perform(sessionsCountBeforeAddingNewOne);
}

This way we crafted the signature of desired session count validation object. We can implement it (test-driven of course) as soon as this spec starts to pass,

That's it. This is a simple lesson, but very important one. Remember: always listen to your tests - the advice may be priceless.

See ya!