As some of you know I kinda blur the lines between DBA and Dev.
It kind reminds me of that half-white/half-black painted guy on that old Star Trek episode sometimes.
Clean Code by Robert C. Martin
In any case, to satisfy my Developer “dark side” cravings I’ve been reading an excellent book called “Clean Code” by Robert C. Martin. This book has some excellent practices to keep your code clean and maintainable.
Most of the book is written in a Java perspective, but it occurred to me this could follow with ANY code, even T-SQL.
So I’m going to attempt doing an ongoing series of tips referencing this book and my coding practice that some might find helpful to maintain their own T-SQL code.
Yes, I know for some this might be bordering on a religious debate, but even if you don’t take some of these practices to heart its food for thought and always sparks an interesting conversation.
Naming for Stored Procedure and Views
By far the most common naming I’ve seen for stored procedures and views is something like “spStoredProcName”, and “vwViewName”. (“sp” designating a stored procedure, and “vw” designating a views).
I am in the camp that thinks this naming convention makes sense. It differentiates a stored procedure and a view so we’re all on the same page both in the DBA and Dev worlds. When the Developer (or DBA) is querying in his/her code they know what they’re dealing with right off the bat based on the prefix.
However, let’s not just stop there with our descriptive naming.
Say I have a procedure that returns all the sales from that day from the database.
Common naming structures I’ve seen are “spReturnSales” or “spGetSales”.
Now let me ask you a question. If you were to open up SSMS and see this sitting in there would you know what it does exactly without opening the procedure? No, right? This could get yesterday’s sales, all sales, or even be tracking returns on sales.
Also, when you query you’re left with something like: EXEC spReturnSales or EXEC spGetSales.
Outside of what sales its returning do we need to supply input parameters (if any)?
Based on that syntax how do you know what you’re querying when you come back to it three months later without having to look at the procedure again?
Make it as descriptive as possible. Encompass your “WHERE” clause
So in the above example lets say it’s returning the sales for today. Then why not name it “spSalesCurrentDay” or “spSalesToday”?
Doesn’t this read better:
EXEC spSalesCurrentDay
There is no confusion on what the data is returning. You know you’re getting the total from sales for the current day, and you’re relatively sure by the name that no inputs are necessary.
AdventureWorks “Refactor”
Let’s “refactor” a stored procedure name in AdventureWorks.
I’m looking at dbo.uspGetEmployeeManagers as an example. Do you see any problems with this name?
Lets dissect this:
First dbo, well that’s the schema so we’ll leave that alone.
Then usp, this stands for “user stored procedure” (as opposed to sp – system stored procedure). It’s my opinion (take it or leave it) that the system stored procedures are in their own folder anyways in SSMS so why have usp at all?
I’m going with “sp” here, but this is a preference thing. Just be consistent.
Now the meat of the name: “GetEmployeeManagers”.
My first questions with this name is which employees? Are they filtered in any way? How about the managers?
First things first. Let’s drop the “Get”. Typically we know we’re “getting” something on a SELECT, it’s redundant.
Now, let’s look at the procedure itself:
[sql]
USE [AdventureWorks]
GO
/****** Object: StoredProcedure [dbo].[uspGetEmployeeManagers] Script Date: 10/29/2011 09:26:37 ******/
SET
ANSI_NULLS
ON
GO
SET
QUOTED_IDENTIFIER
ON
GO
ALTER
PROCEDURE [dbo].[uspGetEmployeeManagers]
@EmployeeID [int]
AS
BEGIN
SET
NOCOUNT
ON;
WITH [EMP_cte]([EmployeeID], [ManagerID], [FirstName], [LastName], [Title], [RecursionLevel])
— CTE name and columns
AS (
SELECT e.[EmployeeID], e.[ManagerID], c.[FirstName], c.[LastName], e.[Title], 0 — Get the initial Employee
FROM [HumanResources].[Employee] e
INNER
JOIN [Person].[Contact] c
ON e.[ContactID] = c.[ContactID]
WHERE e.[EmployeeID] = @EmployeeID
UNION
ALL
SELECT e.[EmployeeID], e.[ManagerID], c.[FirstName], c.[LastName], e.[Title], [RecursionLevel] + 1 — Join recursive member to anchor
FROM [HumanResources].[Employee] e
INNER
JOIN [EMP_cte]
ON e.[EmployeeID] = [EMP_cte].[ManagerID]
INNER
JOIN [Person].[Contact] c
ON e.[ContactID] = c.[ContactID]
)
SELECT [EMP_cte].[RecursionLevel], [EMP_cte].[EmployeeID], [EMP_cte].[FirstName], [EMP_cte].[LastName],
[EMP_cte].[ManagerID], c.[FirstName] AS
‘ManagerFirstName’, c.[LastName] AS ‘ManagerLastName’
— Outer select from the CTE
FROM [EMP_cte]
INNER
JOIN [HumanResources].[Employee] e
ON [EMP_cte].[ManagerID] = e.[EmployeeID]
INNER
JOIN [Person].[Contact] c
ON e.[ContactID] = c.[ContactID]
ORDER
BY [RecursionLevel], [ManagerID], [EmployeeID]
OPTION (MAXRECURSION 25)
END;
[/sql]
Wow! Kinda ugly. But, for our purposes the relevant lines are the input and the output. So we have:
[sql]
@EmployeeID [int]
[/sql]
So this gets an input of @EmployeeID
and
[sql]
SELECT e.[EmployeeID], e.[ManagerID], c.[FirstName], c.[LastName], e.[Title], [RecursionLevel] + 1 — Join recursive member to anchor
[/sql]
This is our select data getting manager information.
It’s also doing some recursion, but for the sake of our name that’s an internal procedure we don’t need to worry about.
So, taking the input and output and the other prefixes here’s the name I came up with: spManagerByEmployeeID (yours may be slightly different but hopefully as descriptive)
Now let’s compare my new name to the original:
“EXEC dbo.uspGetEmployeeManagers” or mine “EXEC dbo.spManagerByEmployeeID”
Right off the bat mine is better. Why?
Well, mine points to the fact that you may have to give the procedure a parameter (EmployeeID). The other procedure mentions nothing about the input, it’s not descriptive.
With mine you may not even have to crack open the procedure to know what inputs to give it.
What if this had multiple inputs? No problem, just append. Could be like this: dbo.spManagerByEmployeeIDAndHireDate. Any ideas what parameters to supply a procedure named this?
This is just the tip of the iceberg.
You may agree or disagree, but I think we can all agree that whatever you decide as along as you are consistent with naming, and as descriptive as possible the easier you make it on yourself (and others) going forward.
