Tag: clean code

Cutting Corners

On By David GreenIn Software developmentLeave a comment

The pressure to deliver yesterday is strong. If it’s not customers nagging you, it’s project managers breathing down your neck or your own self-doubt that this should have been simpler: the desire to get the task done quicker can often be irresistible. How do you strike the right balance between cutting corners and polishing the turd?

While working through a feature I maintain a “navigator pad” of things I want to come back to. These are refactorings I’ve spotted, tests that need cleaning up, design smells to look at or just plain questions I’m curious to know the answer to (can foo ever actually be null? is this method really used?) This list ebbs and flows as I’m working through a feature: some days I seem to do nothing but add new things to it, other days I manage to cross half the list off as some much-needed refactoring becomes critical to complete the next change. But the one constant throughout a feature is the nav pad.

Recently I was nearing the end of a feature and my nav pad didn’t seem to be getting any shorter. I’d spent a good bit of time refactoring things, but new problems kept appearing – it didn’t seem like I’d ever be “Done”. The feature was way behind schedule, my self-doubt was growing: I’m trying to do a good job, I don’t want this to take any longer but I keep spotting things I got wrong before or simply missed. Suddenly one morning, within the space of a couple of hours, I crossed 20 items off the nav pad, sat back and realised: it’s empty! I was Done.

The next thing that struck me was what a strange occurrence this was: I couldn’t remember the last time I’d actually crossed everything off the nav pad. There would always be some last refactorings on the list that on balance could wait until another time; some tidy up that could wait until another day; some question that I no longer cared to know the answer to. But for the first time in a long time, I’d crossed everything off!

Then the doubt sets in: have I over-engineered this? Could I have been done quicker? The pressure to cut corners is really strong: we’re always pushed to be done faster, to do the absolute minimum we can get away with. Yet I know what needs to be done, I know what the problems are with this code: I’ve written them all down in the nav pad. If I don’t fix them now, then when?

A pattern I see all-too-frequently when I come up against a design smell: I can see the design is wrong, the tests are a mess, the production code is a mess; there’s definitely a better way, I just can’t see it at the minute. I park the refactoring on the nav pad. I come back to it later after ticking off a few more parts of the feature, but I still can’t see a way to resolve the design smell. I spend a couple of hours refactoring back and forth – in the end I declare bankruptcy and raise an issue in the issue tracker. If I’m lucky I’ll pick up the issue again in a couple of months, have a half-hearted look at it but realise I can’t remember what I was really thinking at the time and close the issue. More likely after a few months with nobody picking up the issue I’ll quietly close it. My code guilt has been neatly dealt with. But the crap code still remains.

The pressure to cut corners is incredibly strong, that pressure is strongest when you’re facing a particularly difficult design change. You’ve identified a problem in the design, probably made obvious by other changes you’ve made. You’re struggling to correct it, which means it isn’t easy to resolve; but it’s obviously a problem because you’ve already spent time trying to resolve it. That means the next time you come through here you’re going to spot the same problem and hate the you of today for not fixing it. And yet, this moment right now is the clearest you’ve ever understood the problem. If you give up now, you’ll have to reload into memory all the context you’ve got right now – what makes you think you’ll be in less of a rush in six months time? That you’ll have time to re-learn this code? Time to do what should have been done today?

The pressure to be done yesterday is strong, but today is the best you’ve ever understood this code: so use that understanding to leave it better than you found it. If you’ve removed all the sharp edges you saw on your way through then at least you’re leaving the code better than you found it. Tomorrow when you pass this way, you’ll pass through a little quicker, with fewer sharp edges to distract you. But today? Today you have code gardening to do.

Are comments always wrong?

On By David GreenIn Software development3 Comments

A colleague asked me recently:

Why aren’t developers writing comments any more?

He’d been looking through some code his team had written, and couldn’t understand it – he was looking for comments to make sense of the mess, but there were none. Before he challenged the team, he asked my opinion: should developers be writing comments?

Excessive Comments

When I started programming some years ago, I would comment everything, and I mean everything.

    // Add four to x
    x += 4;

My logic at the time was that it was impossible to tell the difference between uncommented clear code and gnarly code you just hadn’t spotted the gnarliness in yet. So I would comment everything, where the absence of a comment meant I’d forgotten – not that it was so trivial as to not warrant mentioning.

No Comments

Eventually I started working with peers who knocked some sense into me, and I immediately halved the number of lines of code I produced. At first it was a shock, but soon I realised that clear code is easier to read if there’s just less noise. And this is all (most) comments become: noise. Sometimes they’re accurate, sometimes they’re not. But you can’t rely on them, you always have to assume they might be wrong. So if the code and the comment seem at odds, you assume it’s the comment that’s wrong and not your understanding of the code (naturally you’re infallible!)

Clean Code

Uncle Bob and the notion of clean code have taken “no comments” to an almost fanatical zeal. But every time I get into an argument with someone about how maybe this time a comment might be justified: Ctrl-Alt-M, enter your comment as the method name and it makes the code more obvious. Every. Damned. Time.

However, the trouble with a zealous argument like this is it gets taken up by asshats and lazy people. It’s too easy to say “if you’d read Clean Coder you’d know you don’t need comments. Quit living in the past, grandpa!”. Uh huh. But your code is still a muddled pile of indecipherable crap. At least with comments there’d be some signposts while I wade through your steaming mess.

Some Comments

The truth is: sometimes comments do help (squeal clean code weenies, squeal!) There are some cases where extracting a method name isn’t sufficient. Sometimes having 20 one line methods in my class does not make it easier to read. The end goal is to produce understandable code. Generally, naming things properly helps. Adding comments that get stale does not. But that doesn’t mean that writing crap code and not commenting is the answer. Don’t use “no comments” as an excuse to leave your code indecipherable by human beings.

For example, sometimes you need to document why you didn’t do something. Maybe there’s a standard way of converting between currencies in your application – which this one time you’ve deliberately not used. A comment here might help future people not refactor away your deliberate choice (even better is baking your decision into a design – some class structure that makes it really obvious). Sometimes a method name really doesn’t do a line of code justice, it’s better to be seen in the context of the lines before and after it – but it really needs some explanation of what you’re doing. This is particularly true when dealing with complex algorithms or mathematical formulae.

Getting the Balance Right

How do you get the balance right? Well, your goal is to make code that other people can understand. The best way to get feedback on that is to ask someone. Either have explicit code reviews or pair program. If another human being has read your code and they could understand it – there’s a better than average chance that most other capable people will be able to read it too.

Growing hairy software, guided by tests

On By David GreenIn Software development2 Comments

Software grows organically. One line at a time, one change at a time. These changes soon add up. In an ideal world, they add up to a coherent architecture with an intention revealing design. But sometimes software just grows hairy – full of little details that obscure the underlying logic. What makes software hairy and how can we stop it?

Hairy code

Generally code starts out clean – brand new, shiny code. But each time you make a change that doesn’t quite fit the original design you add a hair – a small, subtle detail. It doesn’t detract from the overall purpose of the code, it just covers a specific detail that wasn’t thought of originally. One hair on its own is fine. But then you add another, and another, and another. Before you know it, your clean, shiny code is covered in little hairs. Eventually code becomes so hairy you can’t even see the underlying design any more.

Let’s face it, we’re all basically maintenance programmers. How many of us actually work on a genuinely greenfield project? And anyway, soon after starting a greenfield project, you’re changing what went before and you’re back into maintenance land. We spend most of our time changing existing code. If we’re not careful, we spend most of our time adding new hairs.

The simplest thing

When changing existing code, there’s a temptation to make the smallest change that could possibly work. Generally, it’s a good approach. Christ, TDD is great at keeping you focused on this. Write a test, make it pass. Write a test, make it pass. Do the simplest thing that could possibly work. But, you have to do the refactor step. “Red, green, refactor“, people. If you’re not refactoring, your code’s getting hairy. If you’re not refactoring, what you just added is a kludge. Sure, it’s a well tested, beautifully written kludge; but it’s still a kludge.

The trouble is, it’s easy to forgive yourself.

But it’s just a little if statement

It’s just one little change. In this specific case we want to do something subtly different. It may not look like it, but it’s a kludge. You’ve described the logic of the change but not the reason. You’ve described how the behaviour is different, but not why. Congratulations, you just grew a new hair.

An example

Perhaps an example would help right about now. Let’s imagine we work for an online retailer. When we fulfill an order, we take each item and attempt to ship it. For those that are out of stock, we add to a queue to ship as soon as we get new stock.

public class OrderItem {
    public void shipIt() {
        if (stockSystem.inStock(getItem()) > getQuantity()) {
            warehouse.shipItem(getItem(),
                               getQuantity(),
                               getCustomer());
        } else {
            warehouse.addQueuedItem(getItem(),
                                    getQuantity(),
                                    getCustomer());
        }
    }
}

As happens with online retailers, we’re slowly taking over the universe: now we’re expanding into shipping digital items as well as physical stuff. This means that some orders will be for items that don’t need physical shipment. Each item knows whether it’s a digital product or a physical product; the rights management team have created an electronic shipment management system (email to you and me) – so all we need to do is make sure we don’t try and post digital items but email them instead. Well, the simplest thing that could possibly work is:

public class OrderItem {
    public void shipIt() {
        if (getItem().isDigitalDelivery()) {
            email.shipItem(getItem(), getCustomer());
        } else if (stockSystem.inStock(getItem()) >
                       getQuantity()) {
            warehouse.shipItem(gettem(),
                               getQuantity(),
                               getCustomer());
        } else {
            warehouse.addQueuedItem(getItem(),
                                    getQuantity(),
                                    getCustomer());
        }
    }
}

After all, it’s just a little “if”, right?

This is all fine and dandy, until in UAT we realise that we’re showing delivery in 3 days for digital items. That’s not right, so we get a request to show immediate delivery for digital items. There’s a method on Item that calculates estimated delivery date:

public class Item {
    private static final int STANDARD_POST_DAYS = 3;
    public int getEstimatedDaysToDelivery() {
        if (stockSystem.inStock(this) > 0) {
            return STANDARD_POST_DAYS;
        } else {
            return stockSystem.getEstArrivalDays(this) +
                       STANDARD_POST_DAYS;
        }
    }
}

Well, it’s easy enough – each item knows whether it’s for digital delivery or not, so we can just add another if:

public class Item {
    private static final int STANDARD_POST_DAYS = 3;
    public int getEstimatedDaysToDelivery() {
        if (isDigitalDelivery()) {
            return 0;
        } else if (stockSystem.inStock(this) > 0) {
            return STANDARD_POST_DAYS;
        } else {
            return stockSystem.getEstArrivalDays(getSKU()) +
                       STANDARD_POST_DAYS;
        }
    }
}

After all, it’s just one more if, right? Where’s the harm? But little by little the code is getting hairier and hairier.

The trouble is you get lots of little related hairs smeared across the code. You get a hair here, another one over there. You know they’re related – they were done as part of the same set of changes. But will someone else looking at this code in 6 months time? What if we need to make a change so users can select electronic and/or physical delivery for items that support both? Now I need to find all the places that were affected by our original change and make more changes. But, they’re not grouped together, they’ve been spread all over. Sure, I can be methodical and find them. But maybe if I’d built it better in the first place it would be easier?

A better way

This all started with a little boolean flag – that was the first smell. Then we find ourselves checking the state of the flag and switching behaviour based on it. It’s almost like there was a new domain concept here of a delivery method. Say, instead I create a DeliveryMethod interface – so each Item can have a DeliveryMethod.

public interface DeliveryMethod {
    void shipItem(Item item, int quantity, Customer customer);
    int getEstimatedDaysToDelivery(Item item);
}

I then create two concrete implementations of this:

public class PostalDelivery implements DeliveryMethod {
    private static final int STANDARD_POST_DAYS = 3;
    @Override
    public void shipItem(Item item, int quantity,
                         Customer customer) {
        if (stockSystem.inStock(item) > quantity) {
            warehouse.shipItem(item, quantity, customer);
        } else {
            warehouse.addQueuedItem(item, quantity, customer);
        }
    }
    @Override
    public int getEstimatedDaysToDelivery(Item item) {
        if (stockSystem.inStock(item) > 0) {
            return STANDARD_POST_DAYS;
        } else {
            return stockSystem.getEstArrivalDays(item) +
                       STANDARD_POST_DAYS;
        }
    }
}

public class DigitalDelivery implements DeliveryMethod {
    @Override
    public void shipItem(Item item, int quantity,
                         Customer customer) {
        email.shipItem(item, customer);
    }
    @Override
    public int getEstimatedDaysToDelivery(Item item) {
        return 0;
    }
}

Now all the logic about how different delivery methods work is local to the DeliveryMethod classes. This groups related changes together; if we later need to make a change to delivery rules we know exactly where they’ll be.

Discipline

Ultimately writing clean code is all about discipline. TDD is a great discipline – it keeps you focused on the task at hand, only adding code that is needed right now; all the while ensuring you have near complete test coverage.

However, avoiding hairy code needs yet more discipline. We need to remember to describe the intention of our change, not just the implementation. Code is primarily to be read by humans so expressing the reason the code does what it does is much more important than expressing the logic. The tests only ensure your logic is correct, you also need to make sure your code reveals it’s reasoning.

If I had more time I would have written less code

On By David GreenIn Software development23 Comments

In a a blatant rip-off of the T.S Eliot quote: “if I had more time, I would have written a shorter letter” I had a thought the other day, perhaps:

If I had more time, I would have written less code

It seems to me the more time time I spend on a problem, the less code I usually end up with; I’ll refactor and usually find a more elegant design – resulting in less code and a better solution. This is at the heart of the programmer’s instinct that good code takes longer to write than crap. But as Uncle Bob tells us:

The only way to go fast is to go well

How then do we square this contradiction? The way to go fast is to go well; but if I sacrifice quality I can go faster – so I’m able to rush crap out quickly.

What makes us rush?

First off – why do developers try to rush through work? It’s the same old story that I’m sure we’ve all heard a million times:

  • There’s an immovable deadline: advertising has already been bought; or some external event – could be Y2K or a big sports event
  • We need to recognise the revenue for this change in this quarter so we make our numbers
  • Somebody, somewhere has promised this and doesn’t want to lose face by admitting they were wrong – so the dev team get to bend so those who over committed don’t look foolish

What happens when we rush?

The most common form of rushing is to skip refactoring. The diabolical manager looks at the Red, Green, Refactor cycle and can see a 33% improvement if those pesky developers will just stop their meddlesome refactoring.

Of course, it doesn’t take a diabolical manager – developers will sometimes skip refactoring in the name of getting something functional out the door quicker.

It’s ok though, we’ll come back and fix this in phase 2

How many times have we heard this shit? Why do we still believe it? Seriously, I think my epitaph should be “Now working on phase 2”.

Another common mistake is to dive straight in to writing code. When you’re growing your software guided by tests, this probably isn’t the end of the world as a good design should emerge. But sometimes 5 minutes thought before you dive in can save you hours of headache later.

And finally, the most egregious rush I’ve ever seen is attempting to start development from “high level requirements”. As the “inconsequential little details” are added in, the requirements look completely different. And all that work spent getting a “head start” is now completely wasted. You now either bin it and do-over, or waste more precious time re-working something you should never have written in the first place.

Does rushing work?

This is the heart of the contradiction – it feels like it does. That’s the reason we do it, isn’t it? When compelled by the customer to finish quicker, we rush – we cut corners to try and get things done quicker.

It feels like I’m going quicker when I cut corners. If only it wasn’t for all those unexpected delays that wasted my time then I would have been finished quicker. Its not my fault those delays cropped up – I couldn’t have predicted that.

It’s the product manager’s fault for not making the requirements clear

It’s the architect’s fault for not telling me about the new guidelines

Its just unlucky that a couple of bugs QA found were really hard to fix

When you cut corners, you pay for it later. Time not spent discussing the requirements in detail leads to misunderstandings; if you don’t spot it later on, you might have to wait until the show and tell for the customer to spot it – now you’ve wasted loads of time. Time not spent thinking about the design can lead to you going down an architectural blind alley that causes loads of rework. Finally time not spent refactoring leaves you with a pile of crap that will be harder to change when you start work on the next story, or even have to make changes for this one. Of course, you couldn’t have seen these problems coming – you did your best, these things crop up in software don’t they?

But what if you hadn’t rushed? What if rather than diving in, you’d spent another 15 minutes discussing the requirements in detail with the customer? You might have realised earlier that you had it all wrong and completely misunderstood what she wanted. What if you spent 30 minutes round a whiteboard with a colleague discussing the design? Maybe he would have pointed out the flaws in your ideas before you started coding. Finally, what if you’d spent a bit more time refactoring? That spaghetti mess you’re going to swear about next week and spend days trying to unravel will still be fresh in your mind and easier to untangle. For the sake of an hour’s work you could save yourself days.

How much is enough?

Of course, it’s all very well to say we will not write crap any more; but it’s not a binary distinction, is it? There’s a sliding scale from highly polished only-really-suitable-for-academics perfection; to sheer, unmitigated, what-were-you-thinking-you-brain-dead-fuck crapitude.

If spending more time discussing requirements could save time later, why not spend years going through a detailed requirements gathering exercise? If spending more time thinking before coding could save time later, why not design the whole thing down to the finest detail before any coding is done? Finally if refactoring mercilessly will save time, why not spend time refactoring everything to be as simple as possible? Actually, wait, this last one probably is a good idea.

Professional software development is always a balancing act. It’s a judgement call to know how much time is enough.

How long does it take?

When working through a development task I choose to expend a certain amount extra effort over and above how long it takes me to type in the code and test it; this extra time is spent discussing requirements, arguing about the design, refactoring the code etc etc. Ideally I want to expend just enough effort to get the job done as quickly as possible (the lazy and impatient developer). If I expend too little effort I risk being delayed later by unnecessary rework; if I expend too much effort I’ve wasted my time.

However, I don’t know in advance what that optimum amount of effort is – it’s a guess based on experience. But I expect the amount of effort I put in to be within some range of the optimum – the lowest the point on the graph:

All other things being equal, I’d expect the amount of time it actually takes to complete the task to be within some margin based on how close I got to the optimum amount of effort. Sometimes I do too little and waste time with rework. Other times I spend too long – e.g. too much detail in the design that didn’t add any value – so the extra time I spent is wasted.

This is by no means all the uncertainty in an estimate; but this is the difference between me not doing quite enough and having to pay for it later (refactoring, debugging or plain rework); versus doing just enough at every stage so that no effort is wasted and there’s no rework I could have avoided more cheaply. In reality the time taken is likely to be somewhere in the middle: not to great but not too shabby.

There’s an interesting exercise here to consider the impact experience has on this. When I was first starting out I’d jump straight into coding; my effort range would be to the left of the graph: I’d spend most of my time re-writing what I’d done so the actual time taken would be huge. Similarly, as I became more experienced I’d spend ages clarifying requirements, writing detailed designs and refactoring until the code was perfect; now my effort range would be to the right of the graph – I’d expend considerable upfront effort, much of which was unnecessary. Now, I’d like to think, I’m a bit more balanced and try to do just enough. I have no idea how you could measure this, though.

Reducing effort to save time

What happens when we rush? I think when we try to finish tasks quicker, we cut down on the extra effort – we’re more likely to accept requirements as-is than challenge them; we’re more likely to settle for the first design idea we think of; we’re more likely to leave the code poorly refactored. This pulls the effort range on the graph to the left.

To try and get done more quickly, I’ve reduced the amount of effort by “shooting low”: I cut corners and expend less effort than I would have done otherwise.

The trouble is this doesn’t make the best-case any better – I might still get the amount of effort bang on and spend the minimum amount of time possible on this task. However because I’m shooting low, there’s a danger now I spend far too little extra effort – the result is I spend even longer: I have to revisit requirements late in the day; make sweeping design changes to existing code; or waste time debugging or refactoring poorly written code.

This is a classic symptom of a team rushing through work: simple mistakes are made that, in hindsight, are obvious – but because we skipped discussing requirements or skipped discussing the design we never noticed them.

When I reduce the amount of extra effort I put in, rather than getting things done quicker, rushing may actually increase the time taken. This is the counter-intuitive world we live in – where aggressive deadlines may actually make things go more slowly. Perhaps instead I should have called this article:

If I had more time I would have been done quicker