What makes good code good and peer software reviewing at Dev8D

Posted by m.jackson on 17 February 2012 - 1:06pm

Bear.jpgOn Wednesday 15th February at Dev8D 2012 I ran a session on peer software reviewing, which I called Enter the bear-pit - peer software reviewing. This included the latest in our occasional discussions on what makes good code good followed by one-on-one sessions where attendees paired up to review each others' code. After sitting in an empty room for ten minutes, looking at the clock and getting more ever more twitchy, ten attendees arrived to participate.

The attendees, who were software developers, with a couple of researcher-developers, put together a list of the qualities of good code, namely:

  • Documented with usage examples so it's clear not just how to run it but how to use it in your own work.
  • Readable using meaningful naming, using conventions and a consistent style.
  • Optimised.
  • Deodorised  to avoid code smells, that is, code that just feels wrong e.g. large classes or methods, methods with many parameters, duplicated statements in both if and else blocks of conditionals etc. I'd never heard this term before – it has it’s origins in Chapter 3 Bad smells in code by Kent Beck and Martin Fowler in Fowler, Martin (1999). Refactoring. Improving the Design of Existing Code. Addison-Wesley. ISBN 0-201-48567-2. For an overview, see code smells at Wikipedia.
  • Concise and modular avoiding large classes or methods or methods with many parameters.
  • Tested and testable via a test suite.
  • Clear and well-designed so that when you look at it you can understand it and believe it does indeed work.
  • Consistent with the conventions and patterns of its language.
  • Reuses and recycles where appropriate and doesn’t reinvent the wheel.
  • Has copyright and a licence.
  • Well-commented with concise, accurate and up-to-date comments that explain why the code as it is and commented in the recommended style for that language e.g. JavaDoc or Doxygen.
  • Has no commented-out code, since it’s unclear whether such code is redundant, deprecated or should be uncommented in future.
  • Peer reviewed.
  • Usable.
  • Works.
  • Doesn’t silently or cryptically fail.

The attendees were then arranged into twos and threes and spent over an hour discussing each others' software in terms of readability, documentation, tests and general project openness, according to the interests of the individual attendees.

There was discussion at the end on the potentially thorny issue of solo developers who may not want others looking at their code, and might be sensitive, defensive, or aggressive at the suggestion that their code should be reviewed (or changed, or even fixed!) This relates to possible differences between perceived ownership (the individual developer) and its actual ownership (which may be a project or organisation). How is an environment fostered in which code is viewed as being under collective ownership with collective responsibility as to its quality, maintenance and improvement? How best can developers, project leaders or PIs encourage collective ownership, peer review and promote these as benefits in terms of encouraging developer growth and learning, improving code quality and maintainability and reducing risks (such as if a developer is hit by a bus!)? A subject for a future blog post!

From the comments at the end of the session it seemed that the attendees found the session a valuable experience and we hope to run this again at future events.

A big thank you from the Institute to the Dev8D organisers for allowing us to run this session!