What craftsmanship means to me

Over a decade ago now I got my first team lead role. It was a reasonably unexpected promotion when the existing team lead left shortly after I joined. This baptism of fire introduced me to line management, but also made me question my career choice. But it was, in hindsight, the beginning of a new journey: of becoming a software craftsman.

With barely 5 years experience I was certainly no senior developer. And yet, here I had been thrust, into a team lead role. With so little experience I made many, many mistakes and was probably a pretty rubbish boss for the three other guys on the team. I tried my best. But the whole process was very draining. But worse, I started to see programming at a more abstract level. In charge of a team, I could see that all we were was a factory for turning requirements into working code. The entire process began to feel like turning a handle: feed the team requirements, some praise and a little coffee and out comes working code.

In the end, a lot of software ends up being very similar: how many CRUD apps does the world really need? Turns out billions of them. And yet, in conception, they’re not massively exciting. Take a piece of data from the user, shovel it back to the database. Take some data out of the database, show it to the user. All very pedestrian. All very repetitive. In this environment it’s easy to become disillusioned with the process of building software. A pointless handle turning exercise.

I moved on from this baptism of fire to my first proper management role. Whereas previously I was still writing code, now I was effectively a full-time manager. I was the team’s meeting and bullshit buffer. It took a lot of buffering. There was a lot of bullshit. I think we even once managed a meeting to discuss why productivity was so poor: maybe the vast number of meetings I was required to attend each day? Or could it have been the 300 emails a day that arrived in my inbox?

If I was disillusioned with the process of writing software before, I now became disillusioned with the entire industry. A large company, little more than a creche for adults, continuing forwards more out of momentum than anything else. Plenty of emails and meetings every day to stop you from having to worry too much about any of that pesky work business.

It was then that I opened my eyes and saw there was a community outside. That programmers across the world were meeting up and discussing what we do. The first thing I saw was the agile community – but even back then it already looked like a vast pyramid scheme. But I was encouraged that there was something larger happening than the dysfunctional companies I kept finding myself working for.

Then Sandro Mancuso and I started talking about software craftsmanship. He introduced me to this movement that seemed to be exactly what I thought was missing in the industry. Not the agile money-go-round, but a movement where the focus is on doing the job right; on life-long learning; on taking pride in your work.

Not long afterwards Sandro and I setup the London Software Craftsmanship Community, which quickly snowballed. It seems we weren’t alone in believing that the job can be done well, that the job should be done well. Soon hundreds of developers joined the community.

The first immediate consequence of my involvement in the software craftsmanship community was discovering a new employer: TIM Group. A company that genuinely has a focus on software built well, with pair programming and TDD. A company where you can take pride in a job done well. The most professional software organisation I’ve worked in. They’re almost certainly still hiring, so if you’re looking, you should definitely talk to them.

Finally I’d found the antidote to my disillusionment with how software is often built: the reason I was frustrated is that it was being built badly. That companies often encourage software to be built slapdash and without care, either implicitly or sometimes even explicitly. If building software feels like just turning a handle it’s because you’re not learning anything. If you’re not learning, it’s because you’re not trying to get better at the job. Don’t tell me you’re already perfect at writing software, I don’t believe it.

Through software craftsmanship I rediscovered my love of programming. My love of a job done well. The fine focus on details that has always interested me. But not just the fine details of the code itself: the fine details of how we build it. The mechanics of TDD done well, of how it should feel. I discovered that as I became more senior not only did I find I had so much more to learn, but now I could also teach others. Not only can I take pride in a job done well, but pride in helping others improve, pride in their job done well.

Advertisements

Never trust a passing test

One of the lessons when practising TDD is to never trust a passing test. If you haven’t seen the test fail, are you sure it can fail?

traffic-lights-208253_1920Red Green Refactor

Getting used to the red-green-refactor cycle can be difficult. It’s very natural for a developer new to TDD to immediately jump into writing the production code. Even if you’ve written the test first, the natural instinct is to keep typing until the production code is finished, too. But running the test is vital: if you don’t see the test fail, how do you know the test is valid? If you only see it pass, is it passing because of your changes or for some other reason?

For example, maybe the test itself is not correct. A mistake in the test setup could mean we’re actually exercising a different branch, one that has already been implemented. In this case, the test would already pass without writing new code. Only by running the test and seeing it unexpectedly pass, can we know the test itself is wrong.

Or alternatively there could be an error in the assertions. Ever written assertTrue() instead of assertFalse() by mistake? These kind of logic errors in tests are very easy to make and the easiest way to defend against them is to ensure the test fails before you try and make it pass.

Failing for the Right Reason

It’s not enough to see a test fail. This is another common beginner mistake with TDD: run the test, see a red bar, jump into writing production code. But is the test failing for the right reason? Or is the test failing because there’s an error in the test setup? For example, a NullReferenceException may not be a valid failure – it may suggest that you need to enhance the test setup, maybe there’s a missing collaborator. However, if you currently have a function returning null and your intention with this increment is to not return null, then maybe a NullReferenceException is a perfectly valid failure.

This is why determining whether a test is failing for the right reason can be hard: it depends on the production code change you’re intending to make. This depends not only on knowledge of the code but also the experience of doing TDD to have an instinct for the type of change you’re intending to make with each cycle.

When Good Tests Go Bad

A tragically common occurrence is that we see the test fail, we write the production code, the test still fails. We’re pretty sure the production code is right. But we were pretty sure the test was right, too. Eventually we realise the test was wrong. What to do now? The obvious thing is to go fix the test. Woohoo! A green bar. Commit. Push.

But wait, did we just trust a passing test? After changing the test, we never actually saw the test fail. At this point, it’s vital to undo your production code changes and re-run the test. Either git stash them or comment them out. Make sure you run the modified test against the unmodified production code: that way you know the test can fail. If the test still passes, your test is still wrong.

TDD done well is a highly disciplined process. This can be hard for developers just learning it to appreciate. You’ll only internalise these steps once you’ve seen why they are vital (and not just read about it on the internets). And only by regularly practising TDD will this discipline become second nature.

Enterprise class code

There’s a natural instinct to assume that everybody else’s code is an untidy, undisciplined mess. But, if we look objectively, some people genuinely are able to write well crafted code. Recently, I’ve come across a different approach to clean code that is unlike the code I’ve spent most of my career working with (and writing).

Enterprise-class Code

There’s a common way of writing code, perhaps particularly common in Java, but happens in C#, too – that encourages the developer to write as many classes as possible. In large enterprises this way of building code is endemic. To paraphrase: every problem in an enterprise code base can be solved by the addition of another class, except too many classes.

Why does this way of writing code happen? Is it a reaction to too much complexity? There is a certain logic in writing a small class that can be easily tested in isolation. But when everyone takes this approach, you end up with millions of classes. Trying to figure out how to break up complex systems is hard, but if we don’t and just keep on adding more classes we’re making the problem worse not better.

However, it goes deeper than that. Many people (myself included, until recently) think the best way to write well-crafted, maintainable code is to write lots of small classes. After all, a simple class is easy to explain and is more likely to have a single responsibility. But how do you go about explaining a system consisting of a hundred classes to a new developer? I bet you end up scribbling on a whiteboard with boxes and lines everywhere. I’m sure your design is simple and elegant, but when I have to get my head around 100 classes just to know what the landscape looks like – it’s going to take me a little while to understand it. Scale that up to an enterprise with thousands upon thousands of classes and it gets really complex: your average developer may never understand it.

An Example

Perhaps an example would help? Imagine I’m working on some trading middleware. We receive messages representing trades that we need to store and pass on to systems further down the line. Right now we receive these trades in a CSV feed. I start by creating a TradeMessage class.

TradeMessage

private long id;

private Date timestamp;

private BigDecimal amount;

private TradeType type;

private Asset asset;

I’m a good little functional developer so this class is immutable. Now I have two choices: i) I write a big constructor that takes a hundred parameters or ii) I create a builder to bring some sanity to the exercise. I go for option ii).

TradeMessageBuilder


public TradeMessageBuilder onDate(Date timestamp)

public TradeMessageBuilder forAmount(BigDecimal amount)

public TradeMessageBuilder ofType(TradeType type)

public TradeMessageBuilder inAsset(Asset asset)

public TradeMessage build()

Now I have a builder from which I can create TradeMessage classes. However, the builder requires the strings to have been parsed into dates, decimals etc. I also need to worry about looking up Assets, since the TradeMessage uses the Asset class, but the incoming message only has the name of the asset.

We now test-drive outside-in like good little GOOS developers. We start from a CSVTradeMessageParser (I’m ignoring the networking or whatever else feeds our parser).

We need to parse a single line of CSV, split it into its component parts from which we’ll build the TradeMessage. Now we have a few things we need to do first:

  • Parse the timestamp
  • Parse the amount
  • Parse the trade type
  • Lookup the asset in the database

Now in the most extreme enterprise-y madness, we could write one class for each of those “responsibilities”. However, that’s plainly ridiculous in this case (although add in error handling or some attempts at code reuse and you’d be amazed how quickly all those extra classes start to look like a good idea).

Instead, the only extra concern we really have here is the asset lookup. The date, amount and type parsing I can add to the parser class itself – it’s all about the single responsibility of parsing the message so it makes sense.

CSVTradeMessageParser


public TradeMessage parse(String csvline)

private Date parseTimestamp(String timestamp)

private BigDecimal parseAmount(String amount)

private TradeType parseType(String amount)

Now – there’s an issue test driving this class – how do I test all these private methods? I could make them package visible and put my tests in the same package, but that’s nasty. Or I’m forced to test from the public method, mock the builder and verify the correctly parsed values are passed to the builder. This isn’t ideal as I can’t test each parse method in isolation. Suddenly making them separate classes seems like a better idea…

Finally I need to create an AssetRepository:

AssetRepository


public Asset lookupByName(String name)

The parser uses this and passes the retrieved Asset to the TradeMessageBuilder.

And we’re done! Simple, no? So, if I’ve test driven this with interfaces for my mocked dependencies, how many classes have I had to write?

  • TradeMessage
  • TradeType
  • TradeMessageBuilder
  • ITradeMessageBuilder
  • CSVTradeMessageParser
  • Asset
  • AssetRepository
  • IAssetRepository
  • TradeMessageBuilderTest
  • CSVTradeMessageParserTest
  • AssetRepositoryTest

Oh, and since this is only unit tests, I probably need some end-to-end tests to check the whole shooting match works together:

  • CSVTradeMessageParserIntegrationTest

12 classes! Mmm enterprise-y. This is just a toy example. In the real world, we’d have FactoryFactories and BuilderVisitors to really add to the mess.

Another Way

Is there another way? Well, let’s consider TradeMessage is an API that I want human beings to use. What are the important things about this API?

TradeMessage


public Date getTimestamp()

public BigDecimal getAmount()

public TradeType getType()

public Asset getAsset()

public void parse(String csvline)

That’s really all callers care about – getting values and parsing from CSV. That’s enough for me to use in tests and production code. Here we’ve created a nice, clean, simple API that is dead easy to explain. No need for a whiteboard, or boxes and lines and long protracted explanations.

But what about our parse() method? Hasn’t this become too complex? Afterall it has to decompose the string, parse dates, amounts and trade types. That’s a lot of responsibilities for one method. But how bad does it actually look? Here’s mine, in full:

public void parse(String csvline) throws ParseException
{
    String[] parts = csvline.split(",");

    setTimestamp(fmt.parse(parts[0]));
    setTradeType(TradeType.valueOf(parts[1]));
    setAmount(new BigDecimal(parts[2]));
    setAsset(Asset.withName(parts[3]));
}

Now of course, by the time you’ve added in some real world complexity and better error handling it’s probably going to be more like 20 lines.

But, let me ask you which would you rather have: 12 tiny classes, or 4 small classes? Is it better for complex operations to be smeared across dozens of classes, or nicely fenced into a single method?