- Table of Contents
- Microsoft SQL Server Defined
- Microsoft SQL Server Features
- Microsoft SQL Server Administration
- Microsoft SQL Server Programming
- Performance Tuning
- Practical Applications
- Professional Development
- Application Architecture Assessments
- Business Intelligence
- Tips and Troubleshooting
- Additional Resources
Transact-SQL Code Reviews, Part Two
Last updated Feb 11, 2011.
In Part One of this series, I explained what a code review is, what it is used for, and why you should implement one. If you are just joining this series, take a few minutes and check that one out first. This week I’ll finish up the topic and explain some practical tools, processes and steps you can take to do that.
Recall that the primary reasons you need to perform a code review involve four main areas:
- Finding and correcting errors
- Fostering learning
- Increasing team-building
- Distributing “tribal knowledge”
I’ll refer to those goals throughout this article.
To start your code review process, it needs to be important to your organization. It’s going to take time extra time that nobody in the organization feels they have. Last week’s article should go a long way towards helping you make it important.
But for the process to work, and not be just a one-time event it has to be something that you perform regularly. It’s going to take some buy-in and discipline on the part of everyone on the team, so there are a few things you can do as an organization to make that happen.
Baking Code Reviews Into the Culture
The first thing you can do to ensure that a code review gets done is to make it part of the check-in process for code. I’m assuming you have a check-in process, of course, so if you don’t you may need to back up and ensure that you’re doing code-signing, unit tests and the other steps required in any standard development shop. The code review should be one of the checkboxes on this process, so that no developer can check in code without a review, period. At first, this will become a blocker and you should keep it that way. Blockers get dealt with quickly, so hold your ground. After a while, it will become something you just make happen.
The next step you can take to keep the momentum going for the code review is to spread the load as much as possible. In other words, don’t make just one person responsible for looking over everyone else’s code. In fact, even a junior member of the team can look over another person’s code. I’ll explain some tools to help them do that in a moment. In fact, one of the goals of a code review learning is often best accomplished when a junior member of the team looks over other people’s code. The point is, when each person spends a small amount of time on the process, it tends to feel more fair and takes less time both of which ensure that your team will continue to do it.
Finally, keep it professional. Writing a comment that states “What were you thinking? This is awful!” isn’t helpful at all, and will only serve to alienate another person. The point is to being everyone up to a high level of skill, and to ensure the code is the best it can be. So put the ego aside, keep your smart remarks and witty repartee to yourself, and make constructive, objective comments. Keep in mind, this might be the first time someone has gotten this kind of feedback, so be technical, specific and professional in your comments.
Now let’s examine some of the practical steps of the process of a code review, starting at the point where you have been given a set of code in a script to review. Since I’m directing this tutorial series to data professionals, these steps will be a mix of things you see in a high-level programming language and information specific to Transact-SQL.
Begin the process by establishing who will do the code reviews, and a window of time to get them back to the original submitter. You’ll also want to decide if the code Is re-reviewed or checked in after the corrections are made.
To expedite the time, use a checklist. Determine the “target areas” you want to focus on as time goes by, you’ll widen this list, but here’s a quick rundown:
- Functionality Make sure the code does what it says it does. That doesn’t mean a full testing run, but it does mean that you look at the code for basics like INSERTs where they are supposed to be and so on.
- Security Follow a checklist on the proper security aspects for the code, especially if it involves permissions, rights, and role assignments. Look for possible injection attack suspects.
- Performance If the code involves creating an index, make sure it makes sense to do so. If there are query hints or NOLOCK statements, give them extra scrutiny.
- General Refactoring Comment layouts, keywords, variable names and so on are important, but you should focus on query re-writes that might help the performance or readability of the code. Look for BETWEEN statements, inequality comparisons and the like.
General Refactoring is the most difficult and time-consuming area to consider, so you should work up to that. Your first set of code reviews should focus on the functionality, then security and then performance. Once you’ve covered those, then you can start evaluating the rest of the code. Again, this can be useful even if the review is done by someone that is not as expert in an area as the person being reviewed. In fact, it’s a great way to learn. Don’t be afraid to ask questions, even if you think the person you’re reviewing is better than you at something.
Mechanics and Tools
You should set up a standard format for code review comments in the T-SQL file you get for review. I recommend block comments, with a standard way of representing a review comment, who is making the comment, and when. Here’s something you can paste into SQL Server Management Studio (SSMS):
/* Code Review: <ReviewerName, SYSNAME, ReviewerName> Date: <DateReviewed, SYSNAME, DateReviewed> Status: <Status, SYSNAME, Open> Comment: */
Save that in the clipboard, and then paste it wherever you want to make a comment. Add your comments after the “Comment:” part, then press Control-Shift-M when you’re done and add your name and the date and press the OK button.
When you return the code to the originator, they can press Control-F and search for:
to find the items you have commented on. It’s your choice whether or not to leave the comments in-place in the code or remove them. In compiled code, the comments aren’t normally seen by the public, but T-SQL is often stored “in the clear” in a file, so it might be best to remove them when the correction is made.
There are other tools available. In SQL Server Management Studio, if you’re using a “Solution” for your T-SQL code, you can use Bookmarks to annotate the code. Press Control-K and then Control-W to show the bookmark window. From there, you can click the “Add Bookmark” icon to set a line-marker and some text. Bookmarks can be searched, edited and so on, but you will need to do this in the context of a project and solution in SSMS to be effective, so this is less useful if you’re delivered scripts or just statements from a developer’s C# or Java code.
One of the easiest and most universal tools is just to use your favorite text editor to record your comments. Many of them have the replaceable parameter feature as I described for SSMS, but you’ll need to check their documentation to see the process for that tool. The general idea is to add a comment placeholder in a standard way so that the person receiving the review can locate it quickly and knows that it is still current, not an older closed out comment.
There are also formal tools that facilitate the review, but most of them are focused on languages like C++ or C#. If you have one of those tools (check with your developers) just make sure they include you in the process and train you on what they do to perform the review and the routing.
There is a code review book available, but from one of these tool vendors. It does have some interesting whitepapers, though: http://smartbear.com/best-kept-secrets-of-peer-code-review/.