Back to the feature
Step one in a code review: read the code. But what if I told you your review results might improve by prepending a “feature review” step?
Where I work, everybody requests and conducts code reviews. Lately, I noticed my review results differ from the results I get back from co-workers. Most prominently, in my reviews I seem to put:
- more attention to how the overall feature works for end users;
- less attention to the code itself, e.g. the use of design patterns and coding style.
This puzzled me: should my reviews focus less on end users and more on the code itself? My answer turned out to be “no”. The rest of this post explains why.
Literature on code reviews (Wikipedia, 1, 2, 3) does not mention the end user perspective – actually running the application, and trying out the feature to see whether things work as intended.
So, what to call this other part then, a feature review maybe? This turns out to be a term not all too commonly associated with programming. There’s a StackExchange question mentioning it as an addition to code reviews, but no definitive answers there.
To be sure, we do have names for this type of feedback. Two terms pop into my mind:
- Quality assurance (QA) – dedicated testers systematically trying out a feature before delivering to customers and/or end users;
- User acceptance (UA) tests – customers testing and formally accepting (or rejecting) features before putting them into production.
QA and UA are mostly considered as steps taken after code reviews have been performed. While useful as extra safeguards, it’s not the same as including this perspective into the code review process itself.
In lack of a better name, we’ll stick to feature review for the rest of this post and define it as “reviewing a feature’s usefulness from an end user’s perspective”.
Example: user management
How do code reviews and feature reviews relate? This is perhaps best demonstrated with an example. (This example is a simplified version of a real situation encountered at work.)
Say we have an application with a feature request from a customer:
Administrator users should be able to create and activate new (non-admin) users
This feature has been built and is now ready for review. The current implementation is designed as follows:
- Administrator clicks “New user” link on the Users index page
- Fills in form fields (name, e-mail address) and clicks “Save” button
- On success, the system automatically sends an activation e-mail
- A confirmation message is shown on the screen
Reviewing the programming code might reveal things like:
- The code for generating the new user form seems a bit verbose. Consider rewriting the form using a form builder plugin.
- On saving the user, the response is halted until the e-mail has been sent. Consider decreasing the response time by sending the activation e-mail in a background job instead.
Trying out the feature as an administrator might reveal issues such as:
- The “New user” link is positioned below the user list. I already had a couple of test users in my database and couldn’t find the link right away. Maybe move the link to the top?
- I made a typo in the e-mail address and couldn’t find a way to resend the activation mail after fixing it. Maybe we should decouple sending the activation mail from creating the new user?
Is it doing the job?
The code reviews suggests code improvements given the current design. The feature review, on the other hand, suggests design improvements given the requested feature. Put in another way:
- Feature review asks: is it doing the job well?
- Code review asks: is it doing the job cleanly?
For me, the well question should always be answered first: if the current version of the feature is not doing a good job, don’t even bother optimizing the code behind it. Chances are the current code has to be rewritten anyway.
This is also illustrated in the example. The feature review suggests separating creating a user from activating it. This enables administrators to resend activation mails. Adding this functionality should be considered before implementing the code review optimization of sending the e-mail in a background job.
While a code review may suggest it is about code only, such a limited perspective can lead to optimized code that does not work well for end users. Avoid this trap by first reviewing a feature from and end user perspective. Play around and check if everything feels right and works as expected. If so, go ahead and dive into the code for further inspection. If not, fix the feature first – and the code later.
October 10, 2015