One of the biggest annoyances I have with dealing with other people’s code is when they don’t use a sensible style. Obviously there have been many arguments over time about certain aspects of code style (brace placement, indentation, etc.) – but this isn’t really a problem as far as I’m concerned. I couldn’t care less where a given project has the braces placed, or if spaces are used for indentation, as long as it’s clear and consistent. This is something that recently irked me when I came to do some changes on Choob for the first time in what’s probably years.
From what I can tell (I wasn’t on the original team – so only have the code and documentation to go from) there was never a set of style guidelines drawn up when the project started. This, along with people like myself submitting code later through the project, has lead to a mix of code styles being used – and as a result the code is a bit of a mess in places. I’m pretty sure I’m to blame for some of this, so I’d love to fix it, but that’s a bigger problem than it first seems for a couple of reasons; the sheer number of lines of code it would touch, and getting everyone to use the new format going forward.
The first of these problems is less about the logistics of making the change than the problems posed afterwards. If I change the format of every class in the project, I’m going to have my name against pretty much every line of code when viewed in the source control system. Sure, my change was only superficial, but this doesn’t matter when it comes to running “svn blame” to work out who broke what. This is a tricky one, and one I’m not sure there’s a good solution to, other than to make sure that the commit with the format changes in only has those, such that people can then look at the previous revision, which hopefully has the name of the person responsible in it.
Looking at the second problem there’s a whole other set of challenges faced. Choob doesn’t have too many active developers, which I’m not sure would make it easier or more difficult. The numbers being small certainly makes it easier to get the word out, but because of the infrequency with which work is done – it’s entirely possible that they’ll forget that there is now a proper style and eventually the code base ends up back where it started. This can be made easier through IDEs with formatting features, and the ability to distribute a set of rules (I certainly plan to do this with Eclipse if I do reformat the code), but there’s no doubt at least one developer who doesn’t use an IDE, or uses one that hasn’t had the settings created for.
These are both great reasons why you not only should, but need to set out some code style rules at the start of the project. At work we’ve got projects that have strict rules as to how code should be laid out (with the Java ones having approved formatter settings ship with the internal build of Eclipse), and as a result the code base is consistent. Admittedly there are also measures in place, such as code reviews, that wouldn’t be practical in the case of something like Choob (where all the developers are working on it in their spare time around work/university), but having had an agreed style from the start is a huge bonus.
So, what makes a good code style? Well most languages have a set of conventions or even a set of “standards” specified by the creators of the language (for example Sun’s for Java), so in my opinion it makes sense to use these. This way it’s consistent with what developers are likely to find in other code bases (at least, I hope so) and therefore reduces the barrier to entry. There is obviously the problem of ending up with a lot of idiomatic code, but as long as it’s a widely known way of doing things – you should be able to get away with it (Perl’s returning undef for failure for example).
Unfortunately even with this there’s bound to be some argument that comes down to the braces – and I have to admit to there being one element about coding that frequently irritates me with them. Unlike the majority of arguments, however, it’s not about where to put them – but the lack of them when the language makes the optional. Sure for a one line if statement the additional braces can seem like a lot of wasted code, but what happens when a year or two down the line someone comes along and needs to add something? If they’re not careful they can very easily fall into the trap of not adding in the now required braces, and all of a sudden some of the block isn’t conditional. Equally the other side of the argument exists, where a conditional statement is followed by another that is expected to be run regardless, but is indented to the same level – making it impossible to spot at a cursory glance. Having seen both of these it’s never pretty, and for all its sins Perl manages this well be mandating braces in these very situations (obviously Python’s use of whitespace also avoids this problem).
Of course, part of that problem stems from indentation, and I’d hope that requiring sensible indentation goes without saying when discussing good code style. This, when combined with clear code and written in a consistent manner should make it easily maintainable – which is very important in any project that’s actually going to last.
With all of this in mind, I’m currently trying to find a sensible set of format guidelines that work well on the Choob project and can be automated within Eclipse, which I’m hoping I’ll be able to get into the code base and improve the experience for everyone. This is likely to be a bit of effort, and involve looking for the most common code style at the moment, but hopefully I’ll get something sorted this month (and then get nothing but complaints for the months afterwards!)