views:

363

answers:

9

Question: What are some other strategies on avoiding magic numbers or hard-coded values in your SQL scripts or stored procedures?

Consider a stored procedure whose job is to check/update a value of a record based on its StatusID or some other FK lookup table or range of values.

Consider a Status table where the ID is most important, as it's a FK to another table:

alt text

The SQL scripts that are to be avoided are something like:

DECLARE  @ACKNOWLEDGED tinyint

SELECT  @ACKNOWLEDGED = 3   --hardcoded BAD

UPDATE  SomeTable
SET     CurrentStatusID = @ACKNOWLEDGED
WHERE   ID = @SomeID

The problem here is that this is not portable and is explicitly dependent on the hard-coded value. Subtle defects exist when deploying this to another environment with identity inserts off.

Also trying to avoid a SELECT based on the text description/name of the status:

UPDATE  SomeTable
SET     CurrentStatusID = (SELECT ID FROM [Status] WHERE [Name] = 'Acknowledged')
WHERE   ID = @SomeID

Question: What are some other strategies on avoiding magic numbers or hard-coded values in your SQL scripts or stored procedures?

Some other thoughts on how to achieve this:

  • add a new bit column (named like 'IsAcknowledged') and sets of rules where there can be only one row with a value of 1. This would help in finding the unique row: SELECT ID FROM [Status] WHERE [IsAcknowledged] = 1)
A: 

Well to start with, Business Logic should probably be avoided in the storage layer.

As this seems to be unavoidable when using a DB such as Sql Server where a lot of the BL can exists in the DB, I think you might rather revert to using string IDs rather than auto IDs.

This will be a lot more managebale than auto ids, and can be handled in better ways, even when using the actual application layer.

For instance using a .Net approach, a lot of the the unique string IDs can be store anywhere from the config files, to additional lookups using the selected db, XML files.

astander
+1  A: 

If your case is as simple as above, where an IsAcknowledged bit would work, I would go that route. I've never had any problems with that. If you have more complicated scenarios, where you would end up with a dozen bit fields, I don't see any problem using a "magic number" as long as you are fully in control of it. If you are worried about the identity column not mapping correctly when you port the database, you could create another (non-identity) unique column with your ID values, integer or guid or whatever else would be helpful.

Dave
+1  A: 

If the 'Status' entity, which forms part of your domain model, has predefined values, some of which need to be handled in a specific manner by stored procedures, then it is perfectly OK to hardcode references to those specific values in your code. The problem here is that you are confusing what is potentially an abstract key (ID identity column) for a value that has a meaning in your domain model. Whilst it is OK to keep your ID identity column, you should use a meaningful attribute of your domain entity when referring to it in code, this can be the name, or it can be a numeric alias. But this numeric alias should be defined in your domain model, e.g. 3 means 'Acknowledged', and it should not be confused with the abstract ID field which, as you say, may be an identity column in some of your database instances.

AdamRalph
+1  A: 

This is how I would do it. (Because it is much faster than your example)

UPDATE SomeTable
SET CurrentStatusID = [Status].[ID]
FROM SomeTable
 RIGHT JOIN [Status] ON [Name] = 'Acknowledged'
WHERE SomeTable.[ID] = @SomeID

(not tested could have typos)

Hogan
+3  A: 

Don't rely on IDENTITY for all of your IDs. When you have a lookup table that's going to be less than 50 rows for example, it's perfectly reasonable to either define those lookups as having specific IDs or use a string valued code for them. In either case, "hard-coding" is no longer an issue.

Tom H.
+4  A: 

At some level, there's going to be some "hard coding" of values. The idea of eliminating them comes from two sides:

  1. Making the code more readable (i.e., saying 'Acknowledged' rather than 3 is probably going to make your intentions more obvious to the reader.
  2. Making the code more dynamic, where one function can take a parameter rather than several functions that don't (this is a simplification obviously, but the meaning should be fairly self-evident anyway)

Making bit columns for various states can be a good or bad idea; it really just depends on the data. If the data goes through various "stages" (Received, Acknowledged, Under Review, Rejected, Accepted, Responded, etc.) then that approach quickly scales itself out of viability (not to mention the irritating process of having to ensure that only one of the columns is set to 1 at any given time). If, on the other hand, the state is really as simple as you describe, then doing that can make the code more readable and indexes perform better.

The biggest no-no in hard coding values is hard coding values that reference other entities (in other words, hard coding the primary key for a corresponding object). The string 'Acknowledged' is still a hard-coded value, it's just more transparent in its meaning and it isn't a reference to something else. For me, it boils down to this: if you can (reasonably) look it up, do it. If you can't (or if something makes it an unreasonable task either from a performance or maintainability perspective), hard code it. Using this, you can look up the value of 3 by using Acknowledged; you can't look up Acknowledged from anything else.

Adam Robinson
+2  A: 

I recently figured out that magic numbers can be implemented by views:

CREATE VIEW V_Execution_State AS
SELECT 10 AS Pending, 20 AS Running, 30 AS Done

DECLARE @state INT
SELECT @state = Pending FROM V_Execution_State
devio
Pretty cool idea. Like having an enum in SQL. Thanks!
Joey
One problem with this approach though, is that if you decide to add another execution state you now have to not only change the view, but any code which relies on a static structure will be affected.
Tom H.
+3  A: 

For situations like your status table, I create what I call "static" data sets. These tables contain data that

  • Is set and defined upon creation,
  • Never ever changes, and
  • Is ALWAYS the same, from database instance to database instance, with no exceptions

That is, at the same time you create the table, you populate it as well, using a script to ensure that the values are always the same. Thereafter, no matter what where or when the database, you will know what the values are, and can hard-code accordingly and appropriately. (I would never use surrogate keys or the identity column property in these situations.)

You do not have to use numbers, you can use strings -- or binaries or dates, or whatever is simplest, easiest, and most appropriate. (When I can, I use char strings--and not varchars--such as "RCVD", "DLVR", ACKN", and so forth are easier hard-coded values than, say, 0, 2, and 3.)

This system works for non-extensible sets of values. If these values can be modified (such that 0 no longer means "acknowledged", then you have a security access problem. If you have a system where new codes can be added by users, then you have a different and tricky design issue to resolve.

Philip Kelley
Agreed. In these kinds of cases I create a StatusCd column of some sort whose value never changes. This way it can port across systems, yet be understandable just by looking at the code value.
Yoav
+1  A: 

One idea:

CREATE FUNC dbo.CONST_ACKNOWLEDGED()
RETURNS tinyint
AS
BEGIN
   RETURN 3
END

However, it only makes sense if you don't have autonumber, IMHO

gbn