torsdag 10 juni 2010

Writing code is hard work

This post started out as a comment to: http://wolfie.github.com/2010/06/09/method-hierarchies.html
But when i had posted the comment i felt that i had more to say, so here we go:

You do not get clean, readable, maintainable code by just writing short methods, or by just writing some junit tests, or by just refactoring some code now and then. You get clean code by hard work, practise, and discussions.

If you encounter a bug in legacy code, and realise it is in a 1500 line class, you dont just find the problem and fix it. You treat the problem.
You do this by writing unit tests until you have isolated the problem (yes it is possible to write tests for the code, if you dont think so. go buy michael feathers book: http://amzn.to/9q7cNn).
When you have isolated the problem with a failing test, you make the test pass. If you have to change code that is not under test, you write tests for it first. Then you fix the problem, in as small increments as possible, with a run of tests for each increment.
When you have fixed the problem. You have most likely improved the design and readability of the class, if you havent. Do one small change to improve it.


Why should you write small methods? why not just keep theese 4 lines in the other method? its the only place it is used anyway!
For several reasons:
It raises the abstractionlevel to extract methods.
It is easier to click on a methodcall to find what it does, then to ignore 4 lines in a method.
It is easier to verify that 4 lines does what they are expected to do, if they are in a method with a good name. Then just 4 lines among 100 in a big method.
It is easier to write a junit test for a 4 line method, then for 4 lines in a 100 line method.

The problem with your codebase is not that someone wrote crap 4 years ago. It is that you didnt improve it everytime you touched it.
It is not someone elses code that is hard to read, everyone should own the code, it is your code that is hard to read!

I love code reviews, not the code reviews that involve one "chief architect" saying "thats not god enough, fix it". But the code reviews that involve 2-3 people disucssing someones code, why does that method have that name? could we improve a junit test here? could we extract another method? does this class do to many things? and so on. Dont review every single row of code, but thoose lines you review, do it well, discuss it, learn from it, improve from it. The goal of a code review is not perfect code. It is that one year from now, you have improved from it.

I realise now that i havent mentioned TDD or pair-programing. But you already do that anyway, right?

dont forget, software craftmanship is not about speed, it is about quality.

1 kommentar:

  1. Nice blog.

    A class of 1500 LOC is not so much fun to work with, I like to slice up a monster like that if possible.

    SvaraRadera