Is There Value in Having Non-Technical People Do Code Review?
Editorial note: I originally wrote this post for the SmartBear blog. Go to their site and check out the original! If you like this post, there are a lot of good ones there by a variety of authors, on topics like code review, API design, testing, and more.
Here’s a thought exercise for you. Should non-technical people participate in code reviews?
It’s off the beaten path, to be sure, but I think it’s an interesting philosophical consideration. We’re entirely used to code review as an exercise by developers and for developers. But is there a place or purpose for outsiders to review our code?
Why do it?
I’ll state up front my answer to that question: “yes, provided it happens in a specific, directed way.” But to convince you, let me offer some potential benefits that I see, depending on who reviews what.
- In general, it could bring members of the team with different skill sets closer together. Developers learn the business domain; why not let the business people understand the developers’ world?
- This could serve as a sanity check. Are developers writing code that accurately reflects the domain?
- It could also force developers to write code that is cleaner, more readable, and more maintainable. Imagine having to write code that a non-technical person might understand.
I’ll offer more detailed rationale for this thinking shortly. But I imagine you’d agree that these goals would be worth pursuing. If an occasional, different style of code review can help, then it’d be worth doing.
Be careful with this.
But before I talk about what these reviews might look like and how they could help, it’s important to stress that I’m not proposing a radical change to the code review process. What I’m proposing is an occasional exercise to offer a different perspective on the team’s code. Having non-technical folks look at the code shouldn’t be a vehicle for micromanagement or for former techies to quibble over code. It shouldn’t be exhaustive, since a lot of plumbing code will be nonsense to them.
And perhaps above all, this should not be any sort of approval step. The professionals responsible for the technical work product should always have the final say in what the source code looks like. Anything that comes out of the non-technical stakeholder review should simply be food for thought for the development team.
To offer this food for thought, these reviews need to be structured, focused, and productive. They need to provide feedback that helps the developers and the technical work product. Let’s take a look at those opportunities.
You may have heard of the concept of Behavior-Driven Development (BDD). Teams using this approach start feature development by writing failing acceptance tests. They know to stop work on the feature when all acceptance tests are passing.
But these aren’t just any tests. BDD-style acceptance tests are written in prose and contain language specific to the problem domain. This is accomplished through a language called Gherkin that has specific structural rules oriented around formulating tests with “Given X, When Y, Then Z.” An example of such a test might be:
Given a well formed customer order
When I submit the order
Then I receive a receipt
I won’t go too far into specifics, but this English sentence would actually be bound to code in your application, turning this spec/truth statement into an executable test. Perhaps you can see the value in having non-technical folks review these?
What I propose is gathering any business analysts, product owners, or general domain stakeholders together and having reviews of these gherkin tests. No knowledge of programming is required, so this just becomes a discussion about whether or not the developers are accurately capturing the needs of the users. This is an excellent activity for keeping everyone aligned around the correct behaviors.
On top of this activity, you may wish to take things one step further. If your codebase has a “domain layer” (e.g. as exists in Domain Driven Design (DDD))that involves modeling domain concepts as objects, why not occasionally show this code to the same stakeholders?
It’s tempting to think of this as far-fetched, but suspend disbelief for a moment and ask yourself why they shouldn’t be able to understand this. It’s domain modeling code, so it shouldn’t contain database drivers, markup binding paradigms, serialization logic, etc. This should be code where a CustomerInvoice object has AddLineItem() and ComputeTotal() methods. And if these methods are written with clean-self documenting code and are well factored, a non-technical person should be able to more or less follow them.
This review would be less about confirming proper understanding and more about serving as a test of code readability and maintainability. If the BAs, PMs or product owner can make their way through the code, following along and nodding, then it’s a sign of well-factored, maintainable code.
A final kind of non-technical code review that I’ll offer for consideration is having the development team demonstrate pain points or limitations. Instead of telling a project manager or team supervisor that a feature will be slower than expected because it’s “in a bad part of the code,” why not show them?
Sit them down, and show them the 10,000 line class or the 5,000 line method that will need to be changed. Show them the dizzying XML configuration file for your app that’s committed to source control by a different team member once per hour. Invite them to live the pain with you, if just a little bit. Explain to them that a poorly written bit of code with terrible naming looks almost as confusing to you as it does to them. Take them through the file containing hundreds of global variables and explain why it makes the code so hard to change.
The goal of this activity obviously isn’t perfect understanding. Rather, it’s to make sources of slowness more tangible to them so that they understand why some features take a lot longer than others. It can also help you get dispensation to spend time paying down technical debt in legacy code, when this might not otherwise have been granted. A picture, so to speak, is worth a thousand words.
It’s an experiment, to be sure.
I believe there is value in these types of review, but not in the same context as developers performing peer review. These activities are not primarily about catching mistakes or maintaining standards in code, but rather about communication and alignment. All three activities can narrow the gap between the developers and the non-technical folks in general. Review of acceptance tests can help make sure the development team understands the business domain and goals. Having non-techies look at challenges in the code can help the non-technical folks understand that development team’s limtations and pain points. And review of domain code can serve as a sanity check that the developers aren’t writing the kind of code that will lead to more pain points.
These are non-standard activities, but I think you might benefit from experimenting with them, provided you make the goals and expectations clear, and provided all participants buy in. So bring a non-technical person by, and invite him to live in your world for a little while. You both may benefit.