Extreme Coding Makeover: T-SQL Edition

This week I planned to write about deleting records using a CTE. Honest, I was!  However, something happened at work that I wanted to write about while it’s still somewhat fresh in my mind.

Last week (at least I think it was last week… sometimes the days blur together) I rewrote a stored procedure originally written by someone else for one of the projects I’m working on. I completely understand that the procedure was written “quick and dirty” since that person’s time was limited. However, when I saw it, I knew it needed to be rewritten mostly because it’d be easier to maintain in the future. Okay, I’ll admit it! It was bugging the daylights out of me!  

This procedure had over 80 lines of code, and I managed to successfully hack it down to somewhere around 5 lines. Yay me! Feeling pretty good, I made a comment on Twitter about it. After seeing the great feedback I received along with someone’s request for the steps, I thought it’d make a great blog post. So I’m going to show you the main steps I took. Woo hoo! Since I’m feeling awfully generous, I’ll even include a link for the code here so you can download it and play with it to your heart’s content! Yeah, I’m awesome like that or at least that’s what the voices in my head tell me. 😉

Disclaimer: I’ve modified the names of the stored procedure, parameters, tables, columns, and the data to protect the innocent and the not-so-innocent. I’ve also simplified the code I’m presenting to make it easier to see the steps I took since that’s the main purpose of this post. So what you’re seeing is a more basic version. Otherwise, it could distract from the point I’m trying to make.

Hang onto your seats because here we go!  Cue dramatic music…okay, not really.

Even though I changed the name of many objects, I left the syntax pretty much as is including the hard-coded values.  Surely, you must be kidding! No, I’m not kidding and don’t call me Shirley (sorry, couldn’t resist).   So here is the procedure before I overhauled it:

create procedure dbo.getRoyalProjectList
  @StartingProjectNumber int
 ,@ProjectType nvarchar(50) 
 ,@EndingProjectNumber int = null
as
begin
   set nocount on
   if(@EndingProjectNumber IS NULL)
   begin
      if(@ProjectType = 'Evil')
      begin
         select ProjectNumber
               ,ProjectName
               ,@ProjectType ProjectType
               ,ProjectPurpose
         from dbo.RoyalProject
         where ProjectNumber = @StartingProjectNumber
         and   ProjectPurpose = 'Cause Mischief'
      end
      else
      begin
         if(@ProjectType = 'Good')
         begin
            select ProjectNumber
                  ,ProjectName
                  ,@ProjectType ProjectType
                  ,ProjectPurpose
            from dbo.RoyalProject
            where ProjectNumber = @StartingProjectNumber
            and   ProjectPurpose = 'Divine Intervention Needed'
         end
         else
         begin
            if(@ProjectType = 'Not So Evil')
            begin
               select ProjectNumber
                     ,ProjectName
                     ,@ProjectType ProjectType
                     ,ProjectPurpose
               from dbo.RoyalProject
               where ProjectNumber = @StartingProjectNumber
               and  ProjectPurpose = 'Have No Clue'
            end
         end
      end
   end
   else
   begin
      declare @RoyalProjects table
      (
        ProjectNumber  int
       ,ProjectName nvarchar(120)
       ,ProjectType  nvarchar(10)
       ,ProjectPurpose nvarchar(50)
      )
      if(@ProjectType = 'Evil')
      begin
         insert into @RoyalProjects
             (ProjectNumber
             ,ProjectName
             ,ProjectType
             ,ProjectPurpose)
         select ProjectNumber
               ,ProjectName
               ,@ProjectType
               ,ProjectPurpose
         from dbo.RoyalProject
         where ProjectPurpose = 'Cause Mischief'
      end
      else
      begin
         if(@ProjectType = 'Good')
         begin
            insert into @RoyalProjects
               (ProjectNumber
               ,ProjectName
               ,ProjectType
               ,ProjectPurpose)
            select ProjectNumber
                  ,ProjectName
                  ,@ProjectType
                  ,ProjectPurpose
            from dbo.RoyalProject
            where ProjectPurpose = 'Divine Intervention Needed'
         end
         else
         begin
            if(@ProjectType = 'Not So Evil')
            begin
               insert into @RoyalProjects
                  (ProjectNumber
                  ,ProjectName
                  ,ProjectType
                  ,ProjectPurpose)
               select ProjectNumber
                     ,ProjectName
                     ,@ProjectType
                     ,ProjectPurpose
               from dbo.RoyalProject
               where ProjectPurpose = 'Have No Clue'
            end
         end
      end
  
      select ProjectNumber
            ,ProjectName
            ,@ProjectType ProjectType
            ,ProjectPurpose
      from @RoyalProjects
      where ProjectNumber >= @StartingProjectNumber
      and  ProjectNumber <= @EndingProjectNumber
   end
   set nocount off
end
go

So what does it do?

The parameters include:

  • @StartingProjectNumber
  • @EndingProjectNumber (optional)
  • @ProjectType

The @StartingProjectNumber and @EndingProjectNumber parameters allow the user to specify one project number or a range.  The @ProjectType parameter indicates what type of project we’re looking for.

The whole purpose of this procedure is to:

  • Return ProjectNumber values from the RoyalProject table where:
    • the ProjectNumber is between the @StartingProjectNumber and the @EndingProjectNumber
    • OR where the ProjectNumber is equal to the @StartingProjectNumber, if the @EndingProjectNumber is null.
  • Plus, we only want the records where the ProjectPurpose in the table is based on the @ProjectType parameter value.

I included the @ProjectType and ProjectPurpose in the output for debugging purposes only. Note: Since I started writing this post earlier this week, it was decided the @StartingProjectNumber and @EndingProjectNumber parameters were no longer needed and it didn’t really make sense to include them.  However, I decided to leave them in this post since it could potentially help someone in a similar situation.  

If we execute the procedure above like this:

--Retrieve project numbers for evil projects.
declare @StartingProjectNumber int
  ,@ProjectType nvarchar(50) 
  ,@EndingProjectNumber int

--Execute for a range: 
set @StartingProjectNumber = 1
set @ProjectType = 'Evil'
set @EndingProjectNumber = 11

execute dbo.getRoyalProjectList @StartingProjectNumber, @ProjectType ,@EndingProjectNumber;

--Execute for one project number:
set @StartingProjectNumber = 1
set @ProjectType = 'Evil'
set @EndingProjectNumber = null 
execute dbo.getRoyalProjectList @StartingProjectNumber, @ProjectType, @EndingProjectNumber;

The first execute is for a range of  project numbers and the second one is for just one project number.

The results look something like this:

Example Results #1 – Range of Project Numbers

and this:

Example Results #2 – One Project Number

Now that we know what it does, what’s next? First, I tend to look what bugs me the most. In this case, it’s the repetitive select statement.

Step 1: Dude! What’s With All the Selects?

I don’t know about you, but it didn’t really float my boat to see the multitude of selects within all those “if-then” statements. So I looked at all of the selects to determine what is so different about them. In this case, the main differences were:

  • the where clauses contained different hard-coded values depending on the @ProjectType parameter value
  • a table variable is used if the @EndingProjectNumber is not specified.

 Believe it or not, the same thing can be accomplished in one or two select statements without having to use all those “if-then” statements. Applying that logic resulted in this:

create procedure dbo.getRoyalProjectList
  @StartingProjectNumber int
 ,@ProjectType nvarchar(50) 
 ,@EndingProjectNumber int = null
as
begin
   set nocount on
   if(@EndingProjectNumber IS NULL)
   begin
      select ProjectNumber
            ,ProjectName
            ,@ProjectType ProjectType
            ,ProjectPurpose
      from dbo.RoyalProjectBefore
      where ProjectNumber = @StartingProjectNumber
      and ProjectPurpose = (case @ProjectType
                           when 'Evil' then 'Cause Mischief'
                           when 'Good' then 'Divine Intervention'
                           when 'NotSoEvil' then 'Have No Clue'
                           else ''
                           end)
   end
   else
   begin
      declare @RoyalProjects table
      (
        ProjectNumber  int
       ,ProjectName nvarchar(120)
       ,ProjectType  nvarchar(10)
       ,ProjectPurpose nvarchar(50)
      )
  
      insert into @RoyalProjects(ProjectNumber,ProjectName,ProjectType,ProjectPurpose)
      select ProjectNumber
,ProjectName
            ,@ProjectType
            ,ProjectPurpose
      from dbo.RoyalProjectBefore
      where ProjectPurpose = (case @ProjectType
                             when 'Evil' then 'Cause Mischief'
                             when 'Good' then 'Divine Intervention'
                             when 'NotSoEvil' then 'Have No Clue'
else ''
                             end)
      select ProjectNumber, ProjectName, @ProjectType ProjectType, ProjectPurpose
      from @RoyalProjects
      where ProjectNumber >= @StartingProjectNumber
      and  ProjectNumber <= @EndingProjectNumber           
   end
  
   set nocount off
end
go

So what did I do? I combined the “if-then” logic into one case statement and put it into the where clause. While that looks much better now, I’m not quite satisfied yet.  Even though we’re down to two queries now, I’m confidient we can combine these two into one AND eliminate the table variable. Really? Tell me more, oh Princess of SQL!

Step 2:  Is That Table Variable Really Necessary?

So how do we combine these two queries? It’s actually not that difficult. It’s all in the where clause. Remember, what do we ultimately want? We want to

  • Retrieve records where the ProjectNumber is between the @StartingProjectNumber and the @EndingProjectNumber specified
  • OR where the Project Number is equal to the @StartingProjectNumber if the @EndingProjectNumber is null.
  • In addition, we only want the records where the Project Purpose is based on the @ProjectType specified.

What does that look like in T-SQL? I’m glad you asked! It can look something like this:

where (ProjectNumber between @StartingProjectNumber and @EndingProjectNumber
   or (ProjectNumber = @StartingProjectNumber and isnull(@EndingProjectNumber,0) = 0))
and ProjectPurpose = (case @ProjectType
                      when 'Evil' then 'Cause Mischief'
                      when 'Good' then 'Divine Intervention'
                      when 'NotSoEvil' then 'Have No Clue'
                      else ''
                      end)          

The parenthesis are very important here to ensure the Project Number criteria is satisfied depending on whether or not the @EndingProjectNumber was specified. By including this new and improved where clause in my select statement, the revised procedure now looks like this:

create procedure dbo.getRoyalProjectList
  @StartingProjectNumber int
 ,@ProjectType nvarchar(50) 
 ,@EndingProjectNumber int = null
as
begin
    set nocount on

    select ProjectNumber
          ,ProjectName
          ,@ProjectType ProjectType
          ,ProjectPurpose
    from dbo.RoyalProjectBefore
    where (ProjectNumber between @StartingProjectNumber and @EndingProjectNumber
    or (ProjectNumber = @StartingProjectNumber and isnull(@EndingProjectNumber,0) = 0))
    and ProjectPurpose = (case @ProjectType
                          when 'Evil' then 'Cause Mischief'
                          when 'Good' then 'Divine Intervention'
                          when 'NotSoEvil' then 'Have No Clue'
else ''
                          end)          
   set nocount off
end
go

Hey, look at that! We eliminated the table variable AND all of the “if-then” statements! One would think I’d be satisfied with this. Guess again. Yes, I’m just a tad bit demanding. What’s wrong, you ask? Hard-coded values. Sometimes you can’t get around them. In this case, this project is not in production yet, and I actually have some power to make design changes. So guess what? In this situation, I created a new table to map together the Project Type and the Project Purpose. At this point, I replaced the Project Purpose field with an ID field from that new mapping table. 

Step 3: Eliminate Hard-Coded Literals

I generally don’t like to hard-code literals into stored procedures if I can help it. Why not? From the standpoint of future maintenance, I’d much rather modify the values in a table than in a stored procedure. Otherwise, it’s possible the stored procedure would have to be modified every single time the users decide to add more values or change the wording. That could quickly become very time-consuming. In this case, there is a relationship between Project Type and Project Purpose. In this case, it’s just a 1 to Many. One Project Type can have many Project Purposes. Here’s what the data looks like:

Project Type and Purpose Mapping Table

Given the assumption (or per the requirements) that all projects must have a Project Purpose, I can inner join the RoyalProject table on this one which will eliminate the need for a case statement. The newly revisied procedure now looks like this:

create procedure dbo.getRoyalProjectList
  @StartingProjectNumber int
 ,@ProjectType nvarchar(50) 
 ,@EndingProjectNumber int = null
as
begin
   set nocount on
   select RoyalProject.ProjectNumber
         ,RoyalProject.ProjectName
         ,ProjectPurpose.ProjectType
         ,ProjectPurpose.ProjectPurposeDescription
   from dbo.RoyalProject
   inner join dbo.ProjectPurpose
   on  RoyalProject.ProjectPurposeID = ProjectPurpose.ProjectPurposeID
   where (RoyalProject.ProjectNumber between @StartingProjectNumber and @EndingProjectNumber
    or (RoyalProject.ProjectNumber = @StartingProjectNumber and isnull(@EndingProjectNumber,0) = 0))
   and ProjectPurpose.ProjectType = @ProjectType
 
   set nocount off
end
go

Ta da! And there we have it! While it may not be that extreme, I thought it was pretty cool myself. Now I’m sure someone out there is saying “But you said 5 lines! That’s more like 18 lines!” Yeah, I know. Once you remove the Project Name, Project Type and ProjectPurposeDescription from the output, the resulting query itself is closer to 5 lines. Regardless, I think the result is much better than the original. It’s less code to maintain. Now if you’re wondering about performance tuning, well, that’s another post for another day. This week I just wanted to focus on downsizing the code for maintenance purposes. Performance tuning is the next step for another day.

If you’re playing along at home, feel free to execute the different stored procedure revisions to ensure the results are the same. As with pretty much everything, don’t just take my word for it. 🙂 You can download the code here. As with any code you use from my blog or somewhere else, play with it in a development environment just in case. The tables I’ve included are just a basic representation of the real thing. For simplicity, I kept out important things like referential integrity.  Hopefully you’ve learned something you can take with you and apply to your code.

Author’s Note: It turns out that it was a really good thing I rewrote this procedure. Towards the end of the week, I received a request to add more columns  to the output of the procedure that required joins on at least 5 more tables. It may also end up requiring at least one CTE. Can you imagine adding all those joins to the original procedure? *shudder* I think my brain just went numb from just contemplating that one. Well, okay, I’ve seen worse but I still wouldn’t want to keep that original procedure. 😉

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s