Clean Code: what is it?

Recently I helped facilitate some discussion workshops on the topic of Clean Code. Each of the discussions seemed to be predicated on a belief that readability is the most important criterion by which to assess whether code is Clean. Indeed, the groups spent a lot of time discussing ways to establish and police coding standards and suchlike. While I agree that this can be useful, I felt the discussions missed the aspects of Clean Code that I consider to be the most important.

So I thought it might be useful here to attempt to describe what I mean by the term…

Continue reading

Advertisements

TDD for teams

I strongly suspect that TDD for teams is different than TDD for individuals.

There’s a proverb in software development to the effect that:

“TDD is a design technique, not a testing technique”

I agree. But that doesn’t mean it’s the only design technique we need. And it doesn’t also mean that everyone will use it equivalently or get the same results with it. For example, take a look at the approaches used by Seb Rose, Ron Jeffries and Alistair Cockburn to solving the Letter Diamond kata. (Click on their names to read their blog posts, then hop back here if you still have any energy left.) They each tackled the same set of requirements in completely different ways. Each used TDD, and yet their designs were completely different.

In fact, while I was drafting this post, George Dinwiddie had a go too, and Ron made a second attempt. So now we have 5 different design approaches to one toy kata from 4 developers, all in the space of one weekend. (And there are probably more that I haven’t heard about.) Of course, it would have been weird if they had all produced identical designs. We are all different, and that’s definitely a good thing. But I worry that this can cause problems for teams doing TDD.

A couple of years ago I remember doing a performance kata in which I paired with Mark Kirschstein to tackle the supermarket checkout kata. My role was to write the tests and Mark’s was to make them pass. At the beginning of the session I made the bold claim that I could get Mark to implement the checkout in a way he had never seen before, with a design that he had not conceived. The audience, and Mark, were skeptical. They were used to thinking of the problem as framed by the tests in Dave Thomas’ original problem statement. And so they were expecting the first test to be something like this:

public class CheckoutTests {
  @Test
  public void oneA() {
    Checkout checkout = new Checkout();
    checkout.scan("A");
    assertEquals(30, checkout.total());
  }
}

But in fact my first test was this:

public class CheckoutTests2 implements ScannerListener {
  int priceReported = 0;
  String productReported = null;

  @Test
  public void oneA() {
    Scanner scanner = new Scanner(this);
    scanner.scan(new SKU("A"));
    assertEquals(30, priceReported);
    assertEquals("A", productReported);
  }

  public void itemScanned(String product, int price) {
    productReported = product;
    priceReported = price;
  }
}

(expressed using the SelfShunt pattern, as anyone who has attended any of my training courses will recognise immediately). Mark, to his surprise, was gradually led to creating a checkout implementation based on notifications and listeners, and with no getters or setters.

[Supplementary challenge: implement the supermarket checkout kata without conditionals!]

While this was (I hope) great theatre, there’s a deeper message in all of this when it comes to whole teams working on non-trivial problems: If a team is to produce software that exhibits internal consistency and is understandable by all of its developers, then somehow these individual differences must be surrendered or homogenized in some way. Somehow the team must create designs — and a design style — that everyone agrees on.

Does it make sense for teams to operate as isolated pairs, each programming away on their specific tasks, without regard for how their designs will integrate with those of their team-mates? I see too many teams doing just that; ditching design sessions on the basis of reading TDD books and blogs in which a single person or pair did all of the thinking. I see far too many codebases in which personal style is the major design force; where the same domain concept is implemented in two or more radically different ways; where duplication is thus very hard to spot, and even harder to remove.

Perhaps we need more published examples of team-based TDD, showing techniques for creating and sharing Just Enough Design Up FrontTM.

XP includes the key practices of Coding Standard and System Metaphor; are they enough to solve the problem? How can pairs Refactor Mercilessly if there is no team consensus as to what constitutes “good” and “consistent”?

What does your team do to balance the needs of “enough design” and “too much design up front”?

On boolean externalities

This week @avdi wrote a really interesting blog post in which he expounds the woe induced by trying to debug a stack of methods that return booleans to each other. I couldn’t get Disqus to load on the post itself, so my response is below. Go ahead and read Avdi’s article now; take your time, and think about what you would change in his code; I’ll wait here until you’re ready.

A lot of the comments on That Twitter suggest that this can be “fixed” by returning things that are more complex than simple booleans, including switching to a different language to make that easier. I think most of them are missing the point. I think this is a representation problem. Let me explain.

The code as it stands uses several booleans — I counted around six, and I suspect there are more in the code branches we weren’t shown. In order to figure out whether the user is allowed to view an episode, the code effectively rummages through the user’s past, looking for a confluence of events that combine together to give the user that permission. And so when the code doesn’t do what was expected, poor Avdi then has to do the same, only using the debugger or logging statements. The problem therefore, it seems to me, is that the current state of the user is only represented indirectly.

It seems to me that this code breaks the second rule of Simple Design, in that the domain is not represented faithfully by these booleans. I would be willing to bet that not every one of the 2n possible combinations of values of these booleans is possible. Indeed, it seems likely that there are only a handful of states in the lifecycle of a user. So to use a group of booleans to represent these states is to misrepresent the structure of the domain: The representation permits many more possibilities than can actually occur, and leaves the job of figuring out the current state to a bunch of other methods scattered through a couple of different objects. Another way to express this might be to say that the code shows Connascence of Meaning (or Algorithm), because numerous methods need to know something about how the user’s state is represented.

So my solution would be to create an explicit state transition model for users, and then to represent that in the code using a state field on the user. It would then be possible to produce errors such as “User X cannot do Y because s/he is in state Z”. It also opens up the possibility of using the State pattern, so that the user would hold a reference to an object representing its current state. Such an object might know how and when the user arrived in this state, and have code that validates the user’s state changes. It might even provide the valid onward transitions as, say, Command objects.

So that’s my approach, and I’ve been finding it more and more useful recently to solve modelling problems such as this. The question is: is this applicable in Avdi’s case; does the code we haven’t seen support this design…?

Simple Design with Design Patterns

In the blog post Simple Design with Design Patterns, Cory Foy runs a thought experiment in which he explores the use of patterns in solving the Game of Life kata. Cory’s starting point is the intention behind the GoF patterns, coupled with the XP rules of Simple Design and Uncle Bob’s SOLID principles.

Cory stops his analysis when he has the following model:

|Cell| <------- |Status|
           |Alive|    |Dead|

At this point Cory says

“Certainly if we implemented a Cell with a Status object with an AliveStatus class and a DeadStatus class someone, somewhere, should slap us upside the head.”

However, I’d like to carry on from that point just a little further…

I like to teach the use of CRC for exploring design, and so I naturally looked at Cory’s objects from the point of view of their behaviours and responsibilities. Thus it struck me that if we don’t make the split suggested by Cory, we’d have a Cell that knows about its position/neighbours and knows what change to make when the clock ticks. That would violate the Single Responsibility Principle. So the above split into Cell and Status makes sense from the point of view of responsibilities, if we give the Status object the job of deciding how the Cell should change.

With those responsibilities in mind, a little mental CRC session suggests that, when the clock ticks, the Cell would tell its Status object how many neighbours it had, and ask for a new Status object in return.

class Cell
  def change
    @status = @status.update_based_on(@number_of_neighbours)
  end
end

The Status objects become Strategy/Policy objects, and thus don’t seem to need any instance state. All of which means that the Status objects could be Flyweights, with only one extant instance of each type, referenced by every Cell that happens to be in the corresponding state.

I think this approach could well be worth a punt during the upcoming Global Day of Coderetreats

too much TemplateMethod

I’ve been refactoring a lot during the festive break, and I’ve noticed that in many cases it was more difficult than I would have liked. Today I think I figured out the reason for that: I use the TemplateMethod pattern too much.

When I see a duplicated algorithm, it seems that my natural tendency is to push up the skeleton into a superclass. This creates an inheritance relationship within the algorithm, which in turn makes it harder to change. Later, when I do need to change the algorithm, I have to change the superclass and all of the subclasses at the same time. For example, one particular superclass in reek contained three or four template methods, which made the subclasses look quite odd; and each little complex of template-plus-overrides significantly hampered design change in each of the others.

Looking back over my programming career I see that I’ve always had this tendency — I can see it in my old C++, Java and Ruby code. I wonder why? Is it the cost of extra classes, or my mathematical background, or coding habits ingrained before the rise of object-oriented languages? Who knows (and who cares).

So, note to self: Break out State/Strategy objects (and in Ruby this includes Procs) instead of always relying on TemplateMethod.

better after using Rails?

My good friend Clarke Ching asks an interesting question: Could most programmers become better programmers (considerably more productive) by working with Ruby on Rails for a while?

I’m not a huge fan of Rails, and I’ll post more of an answer here later, when I have a little time. In the meantime, what do you think? Can Rails teach everyone to be better?