I am consistently disappointed by most programmers' tendency to mix their UI-logic in the data access layer:
SELECT
FirstName + ' ' + LastName as "Full Name",
case UserRole
when 2 then "Admin"
when 1 then "Moderator"
else "User"
end as "User's Role",
case SignedIn
when 0 then "Logged in"
else "Logged out"
end as "User signed in?",
Convert(varchar(100), LastSignOn, 101) as "Last Sign On",
DateDiff('d', LastSignOn, getDate()) as "Days since last sign on",
AddrLine1 + ' ' + AddrLine2 + ' ' + AddrLine3 + ' ' +
City + ', ' + State + ' ' + Zip as "Address",
'XXX-XX-' + Substring(
Convert(varchar(9), SSN), 6, 4) as "Social Security #"
FROM Users
Normally, programmers do this because they intend to bind their dataset directly to a grid, and its just convenient to have SQL Server format server-side than format on the client.
Queries like the one shown above are extremely brittle because they tightly couple the data layer to the UI layer. On top of that, this style of programming thoroughly prevents stored procedures from being reusable.
Here are my top 3.
Number 1. Failure to specify a field list. (Edit: to prevent confusion: this is a production code rule. It doesn't apply to one-off analysis scripts - unless I'm the author.)
SELECT *
Insert Into blah SELECT *
should be
SELECT fieldlist
Insert Into blah (fieldlist) SELECT fieldlist
Number 2. Using a cursor and while loop, when a while loop with a loop variable will do.
DECLARE @LoopVar int
SET @LoopVar = (SELECT MIN(TheKey) FROM TheTable)
WHILE @LoopVar is not null
BEGIN
-- Do Stuff with current value of @LoopVar
...
--Ok, done, now get the next value
SET @LoopVar = (SELECT MIN(TheKey) FROM TheTable
WHERE @LoopVar < TheKey)
END
Number 3. DateLogic through string types.
--Trim the time
Convert(Convert(theDate, varchar(10), 121), datetime)
Should be
--Trim the time
DateAdd(dd, DateDiff(dd, 0, theDate), 0)
I've seen a recent spike of "One query is better than two, amiright?"
SELECT *
FROM blah
WHERE (blah.Name = @name OR @name is null)
AND (blah.Purpose = @Purpose OR @Purpose is null)
This query requires two or three different execution plans depending on the values of the parameters. Only one execution plan is generated and stuck into the cache for this SQL text. That plan will be used regardless of the value of the parameters. This results in intermittent poor performance. It is much better to write two queries (one query per intended execution plan).
Human readable password fields, egad. Self explanatory.
Using LIKE against indexed columns, and I'm almost tempted to just say LIKE in general.
Recycling SQL-generated PK values.
Surprise nobody mentioned the god-table yet. Nothing says "organic" like 100 columns of bit flags, large strings and integers.
Then there's the "I miss .ini files" pattern: storing CSVs, pipe delimited strings or other parse required data in large text fields.
And for MS SQL server the use of cursors at all. There's a better way to do any given cursor task.
Edited because there's so many!
Don't have to dig deep for it: Not using prepared statements.
Using meaningless table aliases:
from employee t1,
department t2,
job t3,
...
Makes reading a large SQL statement so much harder than it needs to be
var query = "select COUNT(*) from Users where UserName = '"
+ tbUser.Text
+ "' and Password = '"
+ tbPassword.Text +"'";
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With