- 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
- Becoming a DBA
- DBA Levels
- Becoming a Data Professional
- SQL Server Professional Development Plan, Part 1
- SQL Server Professional Development Plan, Part 2
- SQL Server Professional Development Plan, Part 3
- Evaluating Technical Options
- System Sizing
- Creating a Disaster Recovery Plan
- Anatomy of a Disaster (Response Plan)
- Database Troubleshooting
- Conducting an Effective Code Review
- Developing an Exit Strategy
- Data Retention Strategy
- Keeping Your DBA/Developer Job in Troubled Times
- The SQL Server Runbook
- Creating and Maintaining a SQL Server Configuration History, Part 1
- Creating and Maintaining a SQL Server Configuration History, Part 2
- Creating an Application Profile, Part 1
- Creating an Application Profile, Part 2
- How to Attend a Technical Conference
- Tips for Maximizing Your IT Budget This Year
- The Importance of Blue-Sky Planning
- Application Architecture Assessments
- Business Intelligence
- Tips and Troubleshooting
- Additional Resources
Conducting an Effective Code Review
Last updated Jan 20, 2006.
If you're a DBA in a large organization, you may have been asked to conduct a code review of some sort. On the other hand, you may have never been asked to participate in a code review. If you're in a shop where you have any deliverables to another technical team, or if there is more than one DBA in the organization, you should insist that you receive and provide code reviews.
Just what is a code review for or by a DBA? Most developers are familiar with the process where a testing group or perhaps a senior developer looks at their code to ensure the quality of the work. It's sometimes a very informal affair where one developer asks another to check out some snippet of code to make sure it "looks right". In some cases it's a formal process where the developer submits code to a quality group that checks each and every line, with forms, meetings and documented processes. While developers are used to this kind of scrutiny, DBA's are often not as used to having their work checked in this way.
In another article I separated DBA's by their duties: the Maintenance DBA, the Development DBA, and the Data Architect DBA. Many DBA's understand the need for a code review for those who write stored procedure code, views, and other programmable functions in the database. But from the maintenance plans you write to the Extract, Transform and Load procedures a Data Architect creates there's a need to have someone that you're accountable to. Your organization and your career deserve no less.
Items for Review
So what kinds of things should be reviewed by and for the DBA? There are lots of areas that can be reviewed, but the guiding principal is that if the activity affects other processes or code and is not obviously discoverable, it should be subjected to a review. I've written quite a few utilities and scripts that help me through my day at work. When those grow into something that others depend on and what I've developed is outside what is obvious, I ask someone to look it over. Depending on how large that impact is, I'll ask for more people to evaluate it or for a formal review.
Whenever you design a new system or process, you should have another group look at that design. This holds true whether you design in a group or by yourself. This applies to anything you design from Servers to processes.
This means, of course, that you'll have to document. If you're not over that hurdle, consider this your first review: You need to write down or draw out what you're thinking. It's the only way things can be adequately checked, and the only way to prove you've thought it through.
You might think that maintenance plans are a slam-dunk. You follow a few Wizards, check a few logs, and you're good. Don't ever let an old main-framer or Oracle DBA hear you say that. In fact, they are some of the best sources for reviews on your maintenance checks. They'll help you think through things you might have missed.
You normally develop Recovery Plans in a group, but trust me when I say that you need to have the Business teams review your recovery plans for their servers. You'll find that they often have different opinions about the time for the recovery window.
In one case I explained to the business that the recovery time had to include getting replacement parts. They were unaware that their recovery time would be affected by the amount of time it took me to locate, order and procure a replacement drive. Once they understood what was involved, they opened the purse strings a little and allowed me to keep another drive on standby.
T-SQL Statements are a natural candidate for code reviews. Sometimes we forget that the system will call the stored procedure we're writing several times a second. Having someone look over the code we create is vital to ensure that we've got the optimal setup.
If you're the one doing the review, make sure that you consider the code holistically. A particular statement might run fine by itself, but cause massive locking when run in concert with other code.
Anyone who designs a Business Intelligence system will undoubtedly develop it with a group. When you design those systems, it's easy to think that the group development has somehow removed the need for code reviews. But you should have another group review what you plan to do, especially the timetables. Not only will it help you produce a better system, but it will alert others to the impacts you'll make on their systems.
The format of the code review that a DBA faces is less important than the goals for the review. The overall goal is to make the best system possible for your organization. There are four major outcomes that the code review process provides.
The first goal in a code review is to make sure that the code or process follows the stand operating procedures or software lifecycle process at your organization. This of course assumes that you have those, so if you don't you'll need to get those in place first.
Doing things in a standardized way is important for many reasons. For one it makes the code easier to create and check. For another it fosters best practices throughout your organization.
When you're looking at code or a process, make sure that you compare it to the standards your company maintains.
Einstein was attributed this quote: "You do not really understand that which you cannot explain to your grandmother." Although more quotes are attributed to Einstein than he could have possibly stated in his whole lifetime, the thought is sound. I'll often include groups in my review that don't have anything to do with the process I'm working on. I'll wander over to their cube with my documentation and some donuts and ask if I can get as many of them as will listen to me to allow me to explain what I'm doing. I may or may not drill into the details, but I'll watch to make sure they understand what I'm explaining and ask for feedback at the end of the presentation. It's surprising the number of times I've changed my process because a group of non-technical people asked some pretty pertinent questions.
Other than making sure your logic is sound, soliciting a review from someone more experienced than yourself helps you to think of things in a whole new way. If you're right, it enforces the good practices. If you're wrong, you learn to do it the right way. As the old Yiddish proverb says: "When you come to a fork in the road, take the third path."
It's important to have others check your code and processes to make sure they are as fast as possible. The way to do that is to have the code or process only work as hard as they have to and to take the shortest possible path in the work.
For instance, one of the biggest performance tuning tips in a SQL query is to only ask for the data you need. Although this sounds like an obvious statement, I constantly find queries that ask for 100 fields when only 5 are used at the client.
The training benefits of code review are often overlooked. Most technical types are often afraid to look as if they don't know something in front of their peers. Having a consistent code review process for everyone means that the DBA's learn from each other in various areas. No one knows everything, despite what you (or they) might think to the contrary.
Whether you're asking for a review of your work or reviewing someone else's, there are some valuable things you can keep in mind to help with the process. If you're receiving the review, don't take it personal. Anything you learn, so long as you separate your feelings from it, is a good thing. One of the most valuable skills I have is not to worry about what others think when I have a question. I'll ask as many people as will listen to get a lot of information, and then I make my decision on what to do. Just ask, and don't worry about what others might have in their heads - you'll be better off for the knowledge either way.
You may be surprised to find yourself being asked to review someone else's work. The first time that happens you're often a little taken aback and a little proud of your vast knowledge. When you get that feeling, take a deep breath, go lay down, and get over yourself. Then come back and take a detached look at what you're asked to review.
In either case, take a look at the suggestions below to help you through the process, whether it's formal or just a friend stopping by to get some help.
Whether you're giving or receiving the code review, don't take it personally. In some cases I've even administered "blind" reviews, where you don't know who authored the code or process. The code doesn't care who wrote it or how long it's been done that way, just that it gets done properly. Leave your feelings at home, and focus on doing the best job possible.
It's tempting to just casually read through the code or process when it's given to you for review, and pass it along with only a few comments. But to make the process work, you've got to work at it.
Take the review seriously - whether you're giving it or receiving it. The point is to make it better, so treat the process with the care it deserves.
The golden rule applies here: treat others as you would like to be treated. If you find something bad in another's work, note it down professionally with the suggestions of how it might be fixed. Trumpeting others mistakes around the office makes you look like a jerk. Leave your ego at home and help out.
If you're receiving some feedback that appears harsh, let it go. Search for the truth among the bluster, and you'll do that much better the next time. If someone is unprofessional enough to bash you for trying, you'll soon be their boss.
The goal is to make the process better so the organization can do better. Sound corny? Maybe you're looking at it the wrong way. Even if you're not in love with your job, you should want to be a professional for yourself. Getting a better product out does just that. And it can't hurt to learn a few new things along the way.
Informit Articles and Sample Chapters
Diane Altwie talks more about the Software Development process here.
Michael Swanson's got some interesting thoughts on code reviews and ownership here.