SQLServerCentral Article

Write Cleaner T-SQL Using Version Control

,

Using a Version Control System (VCS) sounds like a burden. It is something we need to do for a variety of reasons but also a process that does not seem to help us write code or fix bugs. If we only use a VCS as a way to back up our code, then it is indeed a pure overhead, but actually, there are real benefits. You can see the benefits of tracking history in my previous article, and further, we can use version control to write better, cleaner, more readable code.

This is a typical stored procedure that I see again and again:

/*
               Name:                  usp_DoSomething
               Created:              2012-01-01
               Description:        Retrieves the item details
               Parameters:        @PARAMETER INT - the id or the item you want
               Changes:             2012-04-01 - IN - FIX 877 Added order by
                                    2013-05-92 - GE - Fix 876 Added audit logging
*/create procedure usp_DoSomething (@PARAMETER int)
as
               select top 1 a, b, c, d, e from table where p = @PARAMETER;
               --where p <> @PARAMETER --not sure why this ever worked?? IN 2014-01-01
               --order by j desc
               if 1 = 0
               begin
                              --add auditing table entry
                              insert into audit_log(time, user, p)
                              select getdate(), @@user_name, @PARAMETER;
                              --IN FIX:5433 2014-01-02, removing audit detail logging
                              --declare @audit_id int;
                              --select @audit_id = scope_identity();
                              --insert into audit_detail(log_id, type)
                              --select @audit_id, 123;
               end

If we look at the different parts of the code, we start with the header:

/*
               Name:                  usp_DoSomething
               Created:              2012-01-01 - IN
               Description:        Retrieves the item details
               Parameters:        @PARAMETER INT - the id or the item you want
               Changes:             2012-04-01 - IN - FIX 877 Added order by
                                    2013-05-92 - GE - Fix 876 Added audit logging
*/

Stored procedure definitions already show the name at the top of the definition "create procedure XXX" so we can remove the Name line. If we use source control we can use that to show when and who created the procedure so we can remove the "Created" line.

The description line can be useful, I sometimes see examples in the headers and these again can be useful but I would argue that instead of having some examples in the comment header a full suite of unit tests that show all the possible ways that the procedure can be used is much more useful so we can remove the description.

The parameters, similarly to the name are already at the top of the stored procedure so you can remove them from here.

When we use version control we can remove the list of changes because the changes are all explicitly logged in the VCS. I will point out again here that unless the people who work on the code are very disciplined and also have the ability to never make a single mistake then the list of changes is invariably wrong and shouldn't be trusted.

So by using version control we can now immediately cut down our stored procedure to this:

create procedure usp_DoSomething (@PARAMETER int)
as
               select top 1 a, b, c, d, e from table where p = @PARAMETER;
               --where p <> @PARAMETER --not sure why this ever worked?? IN 2014-01-01
               --order by j desc
               if 1 = 0
               begin
                              --add auditing table entry
                              insert into audit_log(time, user, p)
                              select getdate(), @@user_name, @PARAMETER;
                              --IN FIX:5433 2014-01-02, removing audit detail logging
                              --declare @audit_id int;
                              --select @audit_id = scope_identity();
                              --insert into audit_detail(log_id, type)
                              --select @audit_id, 123;
               end

Now looking at this we have our SELECT, and then we have some commented out code. The commented out code often has a change number with it saying why it was commented out. If instead of commenting out the code you trust version control to keep a valid history of your code then you can simply remove the commented out code so we now end up with:

create procedure usp_DoSomething (@PARAMETER int)
as

               select top 1 a, b, c, d, e from table where p = @PARAMETER;
               if 1 = 0
               begin
                             --add auditing table entry
                              insert into audit_log(time, user, p)
                              select getdate(), @@user_name, @PARAMETER;
                              --IN FIX:5433 2014-01-02, removing audit detail logging
                              --declare @audit_id int;
                              --select @audit_id = scope_identity();
                              --insert into audit_detail(log_id, type)
                              --select @audit_id, 123;
               end

Next up we have an “IF” statement that could never possibly be true: if 1 = 0. What this does is leaves the code but stops it from ever being called. I have genuinely seen code in c# that looked like this:

 if (1=2) {
                              if(1=2){
                                             if(1=2){
                                                            if(1=2){
                                                                           if(1=1){
                                                                                          //stop processs
                                                                           }
                                                            }
                                             }
                              }
               }

Whoever wrote the code was obviously upset at having to write all those if statements and didn't want to delete them, but by trusting the version control tools, this developer could have had a much cleaner method.

So if we take out the code that has been commented out by not being reachable we end up with:

create procedure usp_DoSomething (@PARAMETER int)
as
   select top 1
      a, b, c, d, e
    from table
    where p = @PARAMETER;

This is much more succinct and easier to read and understand. In this case the stored procedure is only a single statement, and in the real world stored procedures are longer and more complicated. However, having commented out code and unreachable code creates confusion and makes our code harder to read, modify and execute.

Rate

1.86 (29)

You rated this post out of 5. Change rating

Share

Share

Rate

1.86 (29)

You rated this post out of 5. Change rating