I work on a development team consisting of about 25 active programmers, working on a very large insurance application that has about half-a-million lines of production Smalltalk code and probably a similar amount of unit and functional test code.
For over 5 years we’ve had a designated team of 5 or 6 developers acting as part-time code reviewers. Someone from this team must review every line of code - including the unit and functional tests - before it gets released to the current shared development stream.
This Code Review team has weekly meetings where issues identified during reviews performed in the previous week are discussed and shared within the team. We’ve seen a substantial improvement in overall code quality during the last 5 years but one of the big payoffs has been that this Code Review team has been exposed to pretty well all major aspects of our complex application. This allows us to be very effective in identifying common problems like duplicated code or code not being implemented in the ideal locations. We’re also well-placed to see opportunities for framework changes that other developers, focusing on solving just the problem-de-jour, might miss.
We have had excellent support from our managers but the success of our Code Review process has been predominantly due to the efforts of the development team itself.
This only works if done properly, and is done early in the process.
A place I used to work at, code reviews were done after you’ve done all of your work and it was supposed to be correct and accurate. If there anything they didn’t like or you’d make some insane decision (because you weren’t supposed to figure it all out yourself and not talk to anyone else) the senior team would complain to management.
As a non native English speaker I’ve been calling this pal programming instead of pair programming for years =)
90% of my everyday work consists of test-driven pair programming developing. I do really little code without my current pal, and I’m getting lazy about it. I enjoy every minute we spend sharing programming achievements and frustrations with someone.
So here is a thought: what is more efficient?
Having two programmers doing the job of one, but ensuring better code thus less debugging time.
Have each one on their code and do some code review sessions.
Obviate childish pairs of lazy irresponsible programmers expending more time in youtube than in their code.
(I would add Asterix and Obelix to that list of famous couples)
Very much disagree with Max - reviewing other people’s work is one of my responsibilities as one of the more experienced team members, and I’d be seriously concerned if it looked to me that they were getting careless, expecting me to save them. If the coder is doing their job properly, the reviewer shouldn’t be doing anything more than suggesting ways something could be done better, not ways it’s been done wrong.
For those review resisters they need to be pointed to the analogy that writing code is like writing a book or even producing a scientific article. An author will get the book edited before publication, a scientist will get their work peer-reviewed before publication.
My team recently took on peer review as part of our general approach to process improvement, and the impact on quality it has had is phenomenal. It uncovered some long standing problems with an approach one developer had used for a couple years, brought the team closer together, and gives everyone an opportunity to learn from each other.
While the initial perception from outside the team doing the reviews was that productivity was lower and a lot of time was spent doing these reviews, at the end of the release cycle the time has been more than recaptured in reduced defects and quicker time to complete regression testing.
If you plan to do this with a team, there is a great tool from Atlassian called Crucible that I would highly recommend. It makes the review process much easier, and can be tied into each CVS commit.
It turns out a lot of the techniques in Karl’s book are unnecessary for a good ROI. Meetings, for example, take a ton of time and don’t contribute much to uncovering bugs.
I’m not going tell much, just… not another cop show
On a serious note, I knew about the power of pair programming and work buddy a long time ago. If anything, the most prominent effect I have experience is how working with somebody else in real time inherently has me paying_attention to the matter at hand with such focus. Left alone, my mind tends to wonder and think about other irrelevant topics (read: procrastination and distraction).
And then now, the real problem - it is just too dang difficult to find a buddy who is interested in the same topics and activties as I am. Even individuals who are self-motivated and competent have their own interests and schedule to attend to.
I really believe in the value of pair-programming, to the point that it is very unlikely that I’ll take up a software position if it isn’t part of their workflow in one way or another. I see it one of the biggest signs that the company has an organizational mindset of Egoless Programming.
Even in my hobbies, I’ve worked with my friend to setup a co-working space / hacker house so that I’ll always have my coding buddy by my side. I’m working on redoing our website to have a proper blogging system (learning Ruby with Jekyll) to share our projects. Right now its just something I threw together to host my resume.
We had hoped to turn our basement in to an electronics work area, but the darn thing kept flooding out. Also had some issues with non-developer roommates moving in / staying on for far longer than expected. Being so, we may be leaving our current location soon.
We’ve been looking into tools to help with remote code-reviews so that we aren’t limited to just the people in the house. Right now the best thing we’ve found is to just share out desktops with Chrome Remote Desktop / Google Hangouts, and making use of Hackpad / Github for code sharing. We’d love a tool that integrates more directly with an IDE. Any recommendations?
“You idiot” I cannot imagine a better way to tank morale and boost turnover. Much better would be: “Why did you take this approach? what problem(s) does it solve? How does this class/trait/field/variable serve that purpose?”
a friend of mine we had the rubber duck signal, every time we had a bug, made the other loot at our code, many times we found the bug ourselves, but having him ask, what does this do? helped a lot, and merge conflicts were a lot easier to solve, luckily our computers were side by side, now it seems similar to the coding buddy, i have to do this again.
