There is plenty of guidance on reviewing code.  (Look at CodeFrisk.Com/Guidance.aspx for some links).   I’ve found that the one absolutely most critical piece of a traditional application never undergoes any type of review.  Most applications live and die by their database, and yet it rarely gets reviewed.  If you’re lucky (although you might not think so), you’ll have a DBA check things over, but they are mostly concerned with performance.  Maintainability and adherence to standards are not at the top of their list.  

I’ve listed some of the things I look at when I review a database:

-Appropriate Normalization

It’s tough to say what is and isn’t appropriate normalization.  You’ll know it when you see it.  Stuff like having a separate table for the US states or one called Gender is usually a red flag of having gone too far.  Conversely, having the same field in multiple tables is usually a sign that normalization hasn’t gone far enough. 

--Is all access through stored procedures

Most development shops have a policy that says, “All data access shall be done through stored procedures”.  If you have that policy, check to make sure this is being adhered to.  An easy way to check this is to deny access to the tables and allow access to the stored procedures.

--Look for premature optimization

I’ve seen times when developers are using Join hints or locking hints in their query.  “If I force the query to use a merge join, it goes 5x faster”.  Meaning it goes 5 times faster on the development box which has far fewer rows and 3 fewer processors than the production machine.  Once it goes the to production machine, there’s a good chance that SQL will decide upon a different plan to execute the statement, making the hint destructive on the production machine

--Are objects secured and scripted

Eventually, when you roll the database into a production environment, access to the database will (or should) be locked down.  Is the development environment the same?  Are the scripts that you have under source control (you do have the database creation scripts under source control don’t you?)  include securing of the object.

--Are updates applied in the same order

I can think of no better way to create an application that suffers from chronic deadlock situations than to have one stored procedure update tables A, B, & C (in that order) and have another stored procedure update tables C, B, & A (in that order).  Updates should always be applied in the same order.  Otherwise, you’re increasing the chance that a row in A will be locked by one stored proc while waiting for a row in table C locked by the other stored proc.  Generally the order is enforced naturally because you have to respect foreign keys, but every now and then I come across this.

--Biblical stored procedures

By biblical, I mean volume, not divinely inspired.  I’m referring to stored procedures that use up more than one printer cartridge if you were to print it.  Nowadays when you see a large stored procedure, it’s one of two things:

1) Embedded business logic – This seems to be a remnant of client server programming where you were forced to put your business logic in the stored procedure.  It’s arguable about whether or the database is the best place to keep your business logic.  Most people (including me) believe it’s not.  Business logic should be kept in a business logic component that validates, enforces and calculates.

2) Poorly defined schema.  Most validations that you see taking place in stored procs could be controlled by using schema constructs like check constraints, triggers, foreign keys and defaults.  Put that type of information in the schema where it can be enforced consistently everywhere rather than burdening the stored procedure.

--Does the Development database match what’s in source control

To reinforce this, database schema objects need to be kept under source control.  Having a database backup plan isn’t sufficient.  Putting the objects in source control allow you to manage changes and releases much more effectively.  Otherwise you’ll be struggling to determine which objects have changed in the development database and need to be deployed, and figuring out who added a field to table.  Keeping the schema under source control used to be a very manual process, but now tools like Visual Studio for Database Professionals make this painless.

--Cloned stored procedures / Views.

This is very common.  When you need data, most developers don’t look to see if there is an existing stored procedure or view that satisfies their needs.   They’ll create a brand new one.  Then you end up with a database that has 3 different stored procedures that get a customer record by its ID.  Now all these duplicated procs have to be maintained.  Best prevention for this is to have a strict naming convention for your procedures. 

Those are some of the quick patterns I look for when reviewing a database.  What are the kind of things that stand out when you look over a database?

Wouldn’t you feel comfortable having your code reviewed by an expert?  Go to CodeFrisk.com to see how I can proofread your code at a reasonable price.