My first experience with peer code review was during a summer job as a junior developer at a medium-sized software house in London which developed point of sale (POS) systems. Code reviews there were organised on a regular basis in a meeting room and the team of developers working on a product would get together and discuss someone’s code. This was over 23 years ago now and the details are somewhat hazy but the experience stayed with me as an example of a good practice and way to both improve the quality of the code base and a way for less experience developers to learn from more experienced ones.
It would be 23 years before I worked at a company that also practised peer code reviews on a consistent basis – I joined VMware UK in October 2011. I have been impressed by VMware’s commitment to quality and one of the ways (VMware also use other best-practices such as continuous integration but I won’t discuss them in this post) this is achieved is by ensuring that no code that has not been peer-reviewed is checked into one of the main branches. Given that VMware’s developers are globally distributed face-to-face meetings would be impractical and slow the rhythm of work so we make use of a web-based tool called Review Board.
In many ways using Review Board is actually better than a face to face meeting as all comments are recorded and reviewers can highlight exactly the lines of code that a comment is discussing. Review Board also tracks changes to the submitted code in response to comments so reviews can see not only how the change modifies the existing code base but how the author has adapted the code in response to comments. Since Review Board is web based bug tracker reports can link to the review request for a proposed fix and commit comments can link back to the review that authorized the commit.
Before I go any further let me describe what I see as the benefits of peer code reviews:
- Flaws are spotted early – Although code reviews are not a substitute for automated tests a second or third pair of eyes looking at the code can spot potential issues and suggest fixes.
- They encourage consistency in the code base – With multiple developers looking at any given piece of code any deviations from house coding conventions and re-implementation of functionality already present in libraries or the code-base are much more likely to be noticed.
- They help developers learn the code base – reviewers can point the author to similar functionality already present that they may have overlooked.
- Less experienced coders can learn from more experienced ones – As well as the opportunity to correct specific errors based on review comments codes can also learn to improve their general coding style using feedback on specific examples of code. Many coders (all?) learn best by doing and the more concrete the example and the closer it relates to a specific tasks then the easier it is to learn and apply what has been learnt.
Anyway, so how does this relate to the London Java Community? Back in August 2011 Ged Byrne and I had an idea for a new meetup for the LJC – our thinking was that the LJC organises technical talks, code dojos and social events (the “developer sessions”) but we don’t spend much time actually talking about code itself in detail. We had the idea of a regular meetup during which people could bring code and discuss it with other developers. Ged also suggested the idea of a monthly challenge in which we’d solicit code that solved a particular problem and analyse it in groups.
Since then we’ve organised five events and I think it’s time to look back and see how things are going – here are links to the previous four events and their respective challenges:
- August 2011 – Pulling Tables from a Word Document. Samir Talwar contributed some Java code written in a functional style to address this challenge.
- October 2011 – We took at look at Peter Norvig’s spelling corrector written in 21 lines of python and challenged people to do better in a JVM lanaguage. Maris contributed a clojure version of the code.
- November 2011 – Suduku solver. Here we analysed Peter Norvig’s suduku solver and Maris’s versions in clojure and scala.
- December 2011 – Java puzzlers. This was a bit of a departure from the normal format in which we had a look at some deceptively simple java code fragments and tried to work out what would happen if they were executed.
- January 2012 – Concurrency. Peter Lawrey talked about his thread affinity library, Maris gave an example of using STM in clojure, Michael Barker (LMAX) talked about how they resolved a concurrency related performance issue with the disrupter and Graham Allan talked about the mutability detector project
We typically start the code shares with a short introductory talk to and discuss the background to the challenge. We’ll then break up into groups of 4-6 people and discuss the month’s code in detail. We deliberately keep things low-tech and use printed copies of the code rather than relying on people bringing laptops and wasting time while people download copies of the code etc. After it looks like the groups have run out of things to talk about (between 30-45 minutes) we’ll then re-convene as a single group and discuss what we’ve learned. We keep things as informal as possible and while we’ll pick on people from each group to give feedback to get things started anyone is free to talk about anything related to the code we’ve just looked at.
For me, it’s the informal group discussions that are the most valuable part of the event and although we talk a lot about the code itself we’ve also had emergent debates on subjects like:
- is clojure (and all other lisps) inherently unreadable?
- what is readable code anyway?
- fluent coding
- should we comment code? and if so under what circumstances? are comments harmful?
Another aspect that I consider valuable is that we deliberately don’t limit ourselves to Java code – as well as other JVM languages (scala & clojure so far) we’ve used examples written in python. Ged and I believe that this not only adds to the variety but exposes developers to other ways of approaching software development.
Update 19/1/2012: I was remiss to not mention our sponsors in the first version of this post. Thoughtworks have been paying for the cost of printing the handouts (and a round of drinks in the bar afterwards), Recworks have been helping Ged and me with much of the administration and organisation and Queen Mary, University of London have provided the venue for all the Code Share events so far.