… or, “Code Reviews for Fun and Profit!”
This is the story of Atlassians jumping at the chance to review computer game logic. I work on documentation for Atlassian’s products, namely FishEye, Crucible and Clover. As a non-programmer, this is challenging. My main problem is that each of these is a powerful tool for elite programmers, and it’s very difficult to place yourself in the shoes of a Wintermute-inspired old-school hacker, who has a strong need for some l33t tools, when this is perhaps the last thing you would have considered for yourself on a given day, between breakfast and morning tea.
In particular, Crucible. Crucible is a tool enabling peer reviews of code, which is of course an ennobling practice that fosters team development and allows spooky, pre-release bug detection. Logical bugs are caught so early, they rarely get a chance to crash anything. Weird!
So, my essential problem is that I don’t have any regular impetus to participate in a code review. I’ve never contributed code in a team of programmers, and I certainly can’t contribute much to a Java code review. So, while I was included in the Atlassian team’s internal code reviews (as a new type of participant, the passenger 😉 ), I never really felt that my familiarity with Crucible was deepening. I wasn’t invested in the work at hand.
As I always try to write documentation from a genuinely realistic experience using the product personally, I decided to set up my own Crucible instance and create a code review that the developers would attend. I’ve dabbled in Flash Actionscript programming, so as an example project I used the source code of a Pac-Man clone that I wrote.
Knowing the cachet of my auditing audience, I was fully expecting the Wrath of Hades to be unleashed, in that particularly cutting tone that the deeply intelligent reserve for the “box-of-hammers” ignorant. I expected the feedback to be short. I expected it to be brutal. In a word, I thought it was going to be nasty.
Most of the comments were sober. Some of them were amusing. But what struck me was the sheer sense of seriousness which the developers applied to the task. For them, code review is not a laughing matter, even when dealing with a half-baked piece of hackery that’s been cobbled together with baling wire and duct tape. They maintained a professional, scholarly tone whilst no doubt stifling gales of laughter. Bravo, Atlassians.
Straight off the bat, my indenting was strafed for being less than ideal.
The structure, after many attacks on its logic, began to resemble Swiss Cheese.
Refactoring was called for.
Some logic needed a visit from the Elegance Fairy.
We learned something about Clamping.
Remnants of old cruft were targeted for extermination. You must run a clean shop floor, when programming code.
They liked the way my attributes and variable names had a certain poetic quality about them.
We had a rare moment of cross-language insight.
Capitalisation of certain letters is important.
One reviewer deferred to the solutions put forth by another reviewer.
The length of certain functions was flagged as smelly. Modularisation suggestions were also a recurring theme.
Finally, mild confusion over bracket placement inspired a moment of mirth.
Thanks for reading. Here’s the Flash game in its compiled form: