5 Tips For Effective ABAP Code Reviews
Peer code reviews are an integral part of most development workflows, and as more and more ABAP teams adopt the agile methodology it has become a standard practice in many SAP development teams as well.
During a code review another member of the team examines your changes to look for mistakes and possible improvements, prior to the piece of development work being submitted for release. While this is a time-consuming activity by nature, it is worth the investment as it has many benefits. It helps with catching bugs earlier, ensures that bad practices don’t make their way into the codebase, and also aids knowledge sharing across the team.
Here are a few simple tips to make the (ABAP) code review process less painful and more effective.
1. Keep it short and take your time
Performing a code review is a mentally taxing task for the reviewer and as with most similar activities, the effectiveness of the process drastically decreases with time. Most studies agree on keeping peer review sessions under an hour and no more than 200-500 lines of code (LOC) should be reviewed in one sitting.
Research also suggests that you have a much higher chance of discovering defects in the code under review if the review speed is kept under 500 LOC/hour.
2. Automate, automate, automate
Developer time is expensive so peer reviews should be focused on evaluating aspects of the code that require human understanding, like clarity, readability, architecture, and logic.
It is highly recommended to set up a development and approval process where style conventions and common errors are verified via automated ABAP code tools like ABAP Test Cockpit, Code Inspector, or ABAPLint. Change management automation can help you to integrate these different tools and enforce their usage in the development workflow.
3. Use a checklist
It is a good idea to keep a common code review to-do list for the entire team to avoid recurring issues. This list could contain best practices for security, performance, and business logic in general. The checklist could also contain those elements from your internal style guide that cannot be checked via automated methods.
It is especially important to have items on the list with regards to omissions, as mistakes where something is entirely missing might be difficult to spot (for example missing locking with SAP enqueue objects, missing AUTHORITY-CHECK statements, etc.).
4. Define goals and internal metrics
The agile mindset is about continuous improvement and the code review process should be no exception. However in order to make the process more useful over time, the team has to decide on what to measure the effectiveness of peer reviews with. Some popular metrics include the average number of bugs found per hour or line of code, or the inspection rate (essentially the speed with which reviews are performed).
Goals regarding the review process should be clear and quantifiable too. This is very organization dependent, but a sensible goal would be to reduce the overall number of defects by a predefined percentage as a result of the peer review process.
5. Be positive and make code reviews part of the culture
If done poorly, pointing out problems in someone else’s code can lead to personal conflicts within the team. You should always provide positive and constructive feedback but also accept criticism and be open to suggestions. Finding bugs in this stage of the development process should not be seen as a negative but as a learning opportunity for the entire team.
The code review process should be embraced by the entire development organization, including management. It should be stressed that defects found during peer review is not a way to evaluate the quality of people’s work, but a collaborative effort to improve overall code quality.
Peer reviews are pretty much standard practice in agile development, which is becoming increasingly popular in ABAP teams, but they certainly aren’t exclusive to an agile approach; any SAP team adopting them is likely to improve the quality of their code.
It’s relatively easy to build peer reviews into your workflow, and if you use the right automation to manage your SAP changes you can even make them a mandatory part of the approval process before changes get out of the dev environment. I’d recommend it.
I'm a big fan of code reviews - first encountered in an engineering company in the late 90s. I implemented in one project I was managing (it's a regulatory authority requirement), but was sad to find that one country was simply creating documents that said "no issues found".
I had a whole load of guidelines, but the summary was
Essentially the goal is for all programmers to code as though if a bug ever needs fixing at 3am on a Sunday morning, the person who has to deal with it is a psychopath who knows where you live.