This code works, but - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

SQL Server Development (2000)

This code works, but

Author

Topic

John T.
Posting Yak Master

112 Posts

Posted - 2003-04-29 : 12:57:27

I am looking for criticism. I am certain it is not efficient as it can be. What better place to find out.
I am sending into the sproc some values. Then I take some values from a table in the db to insert into a record in another table.
Values into sproc + fields from Table1 = fields in Table2
<code>
Create Procedure MainInsert @cap varchar(50),@gno varchar(50),@uns varchar(50), @asel varchar(50),@os varchar(50) AS
Declare @gtime SMALLDATETIME Declare @gm varchar(50)Declare @sp varchar(50)Declare @type varchar(50)Declare @sel varchar(50)Declare @conos SMALLINT Declare @ptime SMALLDATETIME Declare @conuns SMALLINT
SELECT @gtime = GTime FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SELECT @gm = Vis + 'vs.' + Home FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SET @ptime = GETDATE()
SET @conuns = CONVERT(SMALLINT,@uns)
SET @conos = CONVERT(SMALLINT, @os)

IF @asel = 'V'
BEGIN
SET @sp = 'MLB'
SET @type = 'Gm'
SELECT @sel = Vis FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SELECT @os = Vos FROM Table1 WHERE IDNum = CONVERT(SMALLINT, @gno)
END
ELSE IF @asel = 'H'
BEGIN
SET @sp = 'MLB'
SET @type = 'Gm'
SELECT @sel = Home FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SELECT @os = Hos FROM Table1 WHERE IDNum = CONVERT(SMALLINT, @gno)
END
ELSE IF @asel = 'O'
BEGIN
SET @sp = 'MLB'
SET @type = 'T'
SELECT @sel = CONVERT(varchar,Tot) FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SET @sel = @sel + 'o'
SELECT @os = Ov FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END
ELSE IF @asel = 'U'
BEGIN
SET @sp = 'MLB'
SET @type = 'T'
SELECT @sel = CONVERT(varchar,Tot) FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SET @sel = @sel + 'u'
SELECT @os = Un FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END
INSERT INTO Table2 (Cap,GNum,Gm,GTime,Sp,Type,Sel,Os,Uns,PTime,Asel)
VALUES((@cap,@gno,@gm,@gtime,@sp,@type,@sel,@conos,@conuns,@ptime,@asel)
GO
</code>
I know it ain't pretty. Is this spaghetti code?

ValterBorges
Master Smack Fu Yak Hacker

1429 Posts

Posted - 2003-04-29 : 13:06:08

Can you post some ddl and dml so we can test without having to make it up.

John T.
Posting Yak Master

112 Posts

Posted - 2003-04-29 : 13:21:47

Hope this is what you want.
Table1
IDNum SMALLINT auto incrementing
Vis varchar(50)
Vos SMALLINT
Home varchar(50)
Tot DEC(3,1)
Ov SMALLINT
Un SMALLINT
GTime SMALLDATETIME

Table2
Cap varchar(50)
GNum varchar(50)
Gm varchar(80)
GTime SMALLDATETIME
Sp varchar(50)
Type varchar(50)
Sel varchar(50)
Os SMALLINT
Uns SMALLINT
PTime SMALLDATETIME
Asel varchar(50)

ValterBorges
Master Smack Fu Yak Hacker

1429 Posts

Posted - 2003-04-29 : 13:36:13

close enough for the ddl.

It should really be

CREATE TABLE TableX
blaha
blaha

dml should look like

INSERT INTO TABLEX (col1,col23....)
VALUES (x1,x2....)

ValterBorges
Master Smack Fu Yak Hacker

1429 Posts

Posted - 2003-04-29 : 14:16:59

This can be combined into one statement

SELECT @gtime = GTime FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
SELECT @gm = Vis + 'vs.' + Home FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)

SELECT @gtime = GTime, @gm = Vis + 'vs.' + Home
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,gno)

Also why pass the stored proc a varchar when it represents an integer
why not just pass it an integer??
It would remove the convert and make the code more readable.

continuing dissection.....

1st Round of optimization...

SELECT @gtime = GTime, @gm = Vis + 'vs.' + Home
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)

SET @conuns = CONVERT(SMALLINT,@uns)
SET @conos = CONVERT(SMALLINT, @os)

IF @asel = 'V'
BEGIN
SELECT @sp = 'MLB', @type = 'Gm', @sel = Vis, @os = Vos
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END
ELSE IF @asel = 'H'
BEGIN
SELECT @sp = 'MLB', @type = 'Gm', @sel = Home, @os = Hos
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END
ELSE IF @asel = 'O'
BEGIN
SELECT @sp = 'MLB', @type = 'T', @sel = CONVERT(varchar,Tot) + 'o', @os = Ov
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END
ELSE IF @asel = 'U'
BEGIN
SELECT @sp = 'MLB', @type = 'T', @sel = CONVERT(varchar,Tot) + ' u', @os = Un
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)
END

INSERT INTO Table2 (Cap,GNum,Gm,GTime,Sp,Type,Sel,Os,Uns,PTime,Asel)
VALUES((@cap,@gno,@gm,@gtime,@sp,@type,@sel,@conos,@conuns,@ptime,@asel)

Continuing into 2 round to combine statements.......

INSERT INTO Table2 (Cap,GNum,Gm,GTime,Sp,Type,Sel,Os,Uns,PTime,Asel)
SELECT
@cap,
@gno,
@gm = Vis + 'vs.' + Home,
@gtime = GTime,
@sp = 'MLB',
@type =
CASE
WHEN @asel IN ('V','H') THEN 'Gm'
WHEN @asel IN ('O', 'U') THEN 'T'
END,
@sel =
CASE
WHEN @asel IN ('V') THEN Vis
WHEN @asel IN ('H') THEN Home
WHEN @asel IN ('O') THEN CONVERT(varchar,Tot) + 'o'
WHEN @asel IN ('U') THEN CONVERT(varchar,Tot) + ' u'
END,
@os =
CASE
WHEN @asel IN ('V') THEN Vos
WHEN @asel IN ('H') THEN Hos
WHEN @asel IN ('O') THEN Ov
WHEN @asel IN ('U') THEN Un
END,
@conos = CONVERT(SMALLINT, @os),
@conuns = CONVERT(SMALLINT,@uns),
@ptime,
@asel
FROM Table1 WHERE IDNum = CONVERT(SMALLINT,@gno)

For more optimization now we need to now what you're doing, why you're doing it, some sample data, the expected results and what you plan on doing with table2.

Edited by - ValterBorges on 04/29/2003 14:41:50

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources