The Skeptical Methodologist

Software, Rants and Management

Peer Review Teams

Peer reviews accomplish a number of things:

  • They are one of the most cost-effective means of ensuring quality
  • They spread general system knowledge across a team
  • They spread best practices

The ideal peer review size in terms of ensuring quality alone is one person. The marginal benefit of adding more than one person to a review, at least in terms of quality, is low. However, there are effective ways to increase peer review team size if you have different reviewers focus on different things.

You ensure that reviewers have different foci by ensuring they come from different perspectives. I’ve seen three general patterns in perspective.

The Architect

The Architect, in this peer review role, will be a senior or very senior engineer who’s had her hands in a lot of parts of the system. They’ll tend to be a jill of all trades, master of none, and be most interested in larger issues.

They’ll best satisfy the “most cost-effective means of ensuring quality”.

Architects will focus on:

  • Whether or not the code’s external API matches other project’s expectations
  • Whether or not the code abides by a coding and design standard, for instance covering requirements such as
    • Readability
    • Testability
    • Maintainability

Architects also, to a degree, help ‘spread general system knowledge across a team’. They can bring other projects knowledge into a peer review, as well as export project specific knowledge out of this project into other peer reviews.

The Junior Developer

Junior-Senior pairs, which are one pairing pattern that seems to work very well (and which I have begun calling ‘Haseltine Pairs’), also provide another perspective in peer review.

They best satisfy “spreading general knowledge about the system” in peer views.

Specifically, having a junior peer review code will

  • Help them better understand the larger project
  • If they’re part of a Haseltine Pair on the project, help them understand how to debug, test, and document the project

Also important is the Junior Developer’s influence on the process itself. Having juniors on a review, and ensuring they’re confident in asking ‘dumb’ questions, helps code become more maintainable and readable. The Senior now knows she’s writing for an ‘audience’, the junior dev, who may or may not understand everything.

It’s important to remember that we’re all juniors when we first set foot in legacy code, even our own if it has been a few months! Having code written for a junior in mind very much helps keeps things maintainable.

The Language Lawyer

The final person on a great peer review would be what’s been called the ‘Language Lawer’. Language lawyers are your subject matter experts in the programming languages and technologies being used.

An important part of a good peer review is access to supporting documentation such as designs and requirements that help the reviewer understand “what problem is this trying to solve” and “what are the broad swaths of the solution I should be looking for?” This can be done via some supporting documentation, however, ‘lore’ (or informal, baked in knowledge in a team that isn’t documented) is much stronger with the two people listed above. The architect knows the broader problem the code tries to solve, whereas the embedded junior / Haseltine junior has a rough idea of how the design is supposed to work.

What’s left for the language lawyer?

Well, the language lawyer ends up being effective precisely because he doesn’t need formal knowledge of the project to have an impact. Language lawyers are good ‘in the small’ – they know about the APIs, the libraries, the language rules, and idiomatic code. Thus, many of their comments won’t be on why a certain design was used versus something else, but rather, why the actual code was written the way it was.

The Language Lawyer best solves the “spreading best practices” problem by using Peer Reviews as a way to help the author become a better programmer.

They’ll look for:

  • Nitty gritty bugs such as null pointer dereferences
  • Small performance gains that don’t hurt readability
  • Idiomatic code structures and improvements to code’s modularity that takes advantage of a specific language

They also help improve quality, especially in error-prone languages.

Putting it all together

Small teams don’t always have many people to draw on to build the ideal peer review. And even in large teams, you don’t always have the make-up required to get your junior, your architect and your lawyer all in a peer review.

But working with this pattern can help you make quick decisions about who’s needed. If someone is a great programmer in Ruby, then the value added by a Haseltine Junior or Architect is higher than that of a language lawyer.

If you’re most big picture senior person is pushing out some code, getting the big picture perspective probably isn’t going to get you as far as ensuring a language expert is involved.

And if someone’s spitting out code without help, getting a dedicated peer reviewer to play the part of a junior to ask the ‘dumb’ questions is going to help more than an architect or lawyer.

If you can build your ideal team, that’s great. If you can’t, look for what weaknesses remain and cover those to get the most out of your peer reviews.

Advertisements

December 19, 2016 - Posted by | Uncategorized

No comments yet.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: