He knows about cursors, and he's not afraid to use them.
-
This post should be accompanied by a little background story. I work for a relatively small company where we use a custom developed .Net application. This application is not yet finished, so regularly, I have to install a new test version. The install often includes running a few T-SQL scripts that add some new fields, procedures and/or functions to the database. Many a time did I consider posting here... Sometimes several times a day.
From the unfortunate, but never corrected misspellings of table and field names, via integer foreign key values stored in CHARs, to the use of many, many ugly and unneeded subqueries (I have seen update statements that have up to 20 of them), these issues were never quite nasty enough for me to actually go ahead and register. Five minutes ago, I decided it was time when I ran into the UDF you see below, which returns 0 if a client exists in the Clients table and 1 if it doesn't. I'm sure this is the first post of a few, and this is a gem that I didn't want to keep from you. To protect the guilty, I've obfuscated names (and only names!). Got cursor?
CREATE FUNCTION dbo.IsClientNew (@ClientTypeID INT, @ClientNumber AS VARCHAR(50))
RETURNS INT
AS
BEGINDECLARE @Client AS INT
DECLARE ClientCursor CURSOR FOR
SELECT dbo.Client.ClientID FROM dbo.Client
WHERE dbo.Client.ClientTypeID = @ClientTypeID AND dbo.Client.ClientNumber = @ClientNumber
OPEN ClientCursor
FETCH NEXT FROM ClientCursor
INTO @Client
CLOSE ClientCursor
DEALLOCATE ClientCursor
IF @Client <> NULL RETURN 0
RETURN 1ENDPs. I've given up on the formatting. Somebody give me a clue as to how I'm supposed to do that please, for my next post.
-
Looks like the coder is a bit too used to working with variables instead of a much simpler IF EXISTS clause. My guess is he's moved into SQL from whatever his first programming language is and doesn't yet understand that not all languages are the same.
-
That's just wrong... a REAL programmer gets all the results and checks for a condition in the application layer.
Note for the ever present idiots: that was sarcasm
-
If only that was the case. See below, exhibit B (again obfuscated). This procedure has been created by the same person, well before the above UDF.
CREATE PROCEDURE dbo.GetProductType
@ProducerID INT,
@CheckValue INT = 0,
@WhatToReturn VARCHAR(10),
@ReturnValue INT = 0 OUTPUT
ASSET NOCOUNT ON
SET @ReturnValue = 0
-- Yes, it's returning one of two things depending on this variable.
-- It either selects a 'typegroup', of which a producer can only have one...
-- Or it selects the type based on some minvalue and maxvalue.
-- Note the peculiar use of double quotes instead of single quotes.
IF @WhatToReturn = "TYPEGROUP"
BEGIN
DECLARE TypeCursor CURSOR FOR
SELECT TypeGroupID FROM ProductType
WHERE ProducerID = @ProducerID
OPEN TypeCursor
FETCH NEXT FROM TypeCursor
INTO @ReturnValueCLOSE TypeCursor
DEALLOCATE TypeCursor
END
ELSE
BEGIN
-- Yeah, usage of the function EXISTS!
IF EXISTS(SELECT TypeID FROM ProductType WHERE ProducerID = @ProducerID AND MinValue <= @CheckValue AND MaxValue >= @CheckValue)
SET @ReturnValue = (SELECT TypeID FROM ProductType WHERE ProducerID = @ProducerID AND MinValue <= @CheckValue AND MaxValue >= @CheckValue)
ELSE
SET @ReturnValue=0
ENDRETURN @ReturnValue
GO
-
I'm guessing he once read somewhere that cursors were fast. Fast is the buzzword!
Try modifying the procedure and showing it to him. Maybe he'll accept it and learn from it.
-
Way to completely fail on the usage of IF EXISTS! Running the same query twice!
Someone needs to teach that guy SELECT INTO...
Though if it were me, I'd tell him to stop being an idiot and do this very trivial SQL code in the application itself.
-
Hey, at least he didn't use a cursor...
-
How about this version? My assumption is that ClientID values are unique in the Client table.
CREATE FUNCTION dbo.IsClientNew (@ClientTypeID INT, @ClientNumber AS VARCHAR(50))
RETURNS INT
AS
BEGINSELECT count( dbo.Client.ClientID )
FROM dbo.Client
WHERE dbo.Client.ClientTypeID = @ClientTypeID AND dbo.Client.ClientNumber = @ClientNumberEND
-
My function has the output backwards, here is the corrected version.
CREATE FUNCTION dbo.IsClientNew (@ClientTypeID INT, @ClientNumber AS VARCHAR(50))
RETURNS INT
AS
BEGINSELECT (1 - count( dbo.Client.ClientID )) AS Exists
END
FROM dbo.Client
WHERE dbo.Client.ClientTypeID = @ClientTypeID AND dbo.Client.ClientNumber = @ClientNumber
-
@DOA said:
That's just wrong... a REAL programmer gets all the results and checks for a condition in the application layer.
Note for the ever present idiots: that was sarcasm
You mean, that's not the way that you're supposed to do things?
How else would you limit your results from SQL when it only supports SELECT * FROM <database>?
-
@Welbog said:
Looks like the coder is a bit too used to working with variables instead of a much simpler IF EXISTS clause. My guess is he's moved into SQL from whatever his first programming language is and doesn't yet understand that not all languages are the same.
Heck, even a plain old standard 'select count(*)' would do! Considering that's one of the first things you learn when using SQL, I don't know what this guy is smoking!
-
@dmitriy said:
My function has the output backwards, here is the corrected version.
CREATE FUNCTION dbo.IsClientNew (@ClientTypeID INT, @ClientNumber AS VARCHAR(50))
RETURNS INT
AS
BEGINSELECT (1 - count( dbo.Client.ClientID )) AS Exists
FROM dbo.Client
WHERE dbo.Client.ClientTypeID = @ClientTypeID AND dbo.Client.ClientNumber = @ClientNumberEND
Yes, ClientID is unique for a give type and number. How about this:
CREATE FUNCTION dbo.IsClientNew (@ClientTypeID INT, @ClientNumber AS VARCHAR(50))
RETURNS INT
AS
BEGINRETURN ISNULL((SELECT 0 FROM dbo.Client WHERE dbo.Client.ClientTypeID = @ClientTypeID AND dbo.Client.ClientNumber = @ClientNumber), 1)
END