Who needs stored procedures anyway? - SQL Server Forums

Home | Weblogs | Forums

Site Sponsored By: SQLDSC - SQL Server Desired State Configuration

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

Site Related Forums

The Yak Corral

Who needs stored procedures anyway?

Author

Topic

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-16 : 08:52:50

I've been asked in no uncertain terms to remove a stored procedure that I setup for insertion into a fairly simple table I have. The justification being that the code I had was far too complex (too many lines) and therefore not maintainable. (stop giggling at the back

)

So just for a laugh I thought we could have a little competition to make the most complete (and complicated but usable

) code to insert into the following table structure (preferably using vb or c#)

CREATE TABLE [dbo].[Errors] (
[ErrorID] [int] IDENTITY (1, 1) NOT NULL ,
[CallingFunction] [varchar] (100) NULL ,
[DateStamp] [datetime] NULL ,
[Message] [varchar] (500) NULL ,
[MessageID] [varchar] (50) NULL ,
[Source] [varchar] (100) NULL ,
[Status] [varchar] (50) NULL,
[ExternalRefID] [varchar] (100) NULL,
[CRMRefID] [varchar] (100) NULL,
[BackOfficeID] [varchar] NULL
) ON [PRIMARY]

(This is on a SQL server 2000 if it matters)

We need to make sure that the insert string correctly contains all the possible fields from the table (all fields are optional)

The code with the least number of lines will be the winner and may even get used in this application. Any response will be judged next monday

For reference the current function that uses a stored procedure is below - anything with more lines will be discounted (as being unmaintainable

)

Private Function DatabaseStore() As Integer?
' returns the ErrorID

Dim dbConnString As String
Dim cmd As New SqlClient.SqlCommand

dbConnString = Trim(System.Configuration.ConfigurationManager.AppSettings("DbConnString"))
If dbConnString <> "" Then
Try
' set command details
With cmd
' setup the sproc call
.CommandType = CommandType.StoredProcedure
.CommandText = "usp_InsertErrorLog"
.Connection = New SqlConnection(dbConnString)

' now set the parameters
.Parameters.Add(New SqlParameter("@CallingFunction", SqlDbType.VarChar, 100))
.Parameters("@CallingFunction").Value = Me.CallingFunction

.Parameters.Add(New SqlParameter("@DateStamp", SqlDbType.DateTime))
.Parameters("@DateStamp").Value = Me.DateTime

.Parameters.Add(New SqlParameter("@Message", SqlDbType.VarChar, 500))
.Parameters("@Message").Value = Me.Message

.Parameters.Add(New SqlParameter("@MessageID", SqlDbType.VarChar, 50))
If Me.MessageID Is Nothing Then
.Parameters("@MessageID").Value = System.DBNull.Value
Else
.Parameters("@MessageID").Value = Me.MessageID
End If

.Parameters.Add(New SqlParameter("@Status", SqlDbType.VarChar, 50))
.Parameters("@Status").Value = Me.Status

.Parameters.Add(New SqlParameter("@Source", SqlDbType.VarChar, 100))
.Parameters("@Source").Value = Me.Source

End With

Dim result As Integer
' make the call and return the value
cmd.Connection.Open()
result = cmd.ExecuteScalar
cmd.Connection.Close()

Return result

Catch ex As Exception
' error occured

Return Nothing
End Try
End If
End Function

-----------

ASCII and ye shall receive.

Arnold Fribble
Yak-finder General

1961 Posts

Posted - 2009-06-16 : 09:07:28

What does Parameters.Add return? It seems a bit bizarre to create a parameter and then have to look it up just to set its value.

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-16 : 09:53:03

it could equally be coded as for example

Parameters.Add(New SqlParameter("@Source", SqlDbType.VarChar, 100)).Value = "toasters"

it adds a parameter object to the parameters collection. This collection is a member of the SQL command object (cmd), so in the code above it effectively passes these parameters to the stored procedure (that I am not allowed to use)

Maybe this is just confusing the issue, what I am hoping for is some horrible string concatenation containing the insert statement. It will need to cater for strings that contain speech marks or single quotes etc.

-----------

ASCII and ye shall receive.

blindman
Master Smack Fu Yak Hacker

2365 Posts

Posted - 2009-06-16 : 10:59:53

I don't understand. Why are you being told to remove the stored procedure because the code that CALLS the stored procedure is too long?

________________________________________________
If it is not practically useful, then it is practically useless.
________________________________________________

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-16 : 11:49:12

One reason is that the calling code is too many lines which is seen as being too hard to maintain and apparently requires specialist knowledge!

The other is that I have been told to use a straight insert i.e. to specifically avoid using a stored procedure. I have coded an insert string and have to use that.

I explained some of the advantages of using a stored procedure for inserts but this was dismissed.

I just wanted something really long and horrible for the insert string (but I wanted it to be something that worked)

-----------

ASCII and ye shall receive.

webfred
Master Smack Fu Yak Hacker

8781 Posts

Posted - 2009-06-16 : 11:53:20

This is wasting my time because I can not see anything unmaintainable.

No, you're never too old to Yak'n'Roll if you're too young to die.

mcrowley
Aged Yak Warrior

771 Posts

Posted - 2009-06-16 : 12:20:02

Maybe you work for this company?

http://thedailywtf.com/Articles/Simple-SQL.aspx

DonAtWork
Master Smack Fu Yak Hacker

2167 Posts

Posted - 2009-06-16 : 13:47:15

wtf, where is whitefang when you need him???

[Signature]For fast help, follow this link:
http://weblogs.sqlteam.com/brettk/archive/2005/05/25.aspx
Learn SQL or How to sell Used Cars
For ultra basic questions, follow these links.
http://www.sql-tutorial.net/
http://www.firstsql.com/tutor.htm
http://www.w3schools.com/sql/default.asp

webfred
Master Smack Fu Yak Hacker

8781 Posts

Posted - 2009-06-17 : 01:35:17

quote:
Originally posted by DonAtWork

wtf, where is whitefang when you need him???

No, you're never too old to Yak'n'Roll if you're too young to die.

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-17 : 04:39:05

quote:
This is wasting my time because I can not see anything unmaintainable.

Then webfred I'm afraid you could never work for my boss, he is effectively telling me that stored procedures are unmaintainable

-----------

ASCII and ye shall receive.

webfred
Master Smack Fu Yak Hacker

8781 Posts

Posted - 2009-06-17 : 05:01:40

Maybe that should make me lucky!
But btw I have not seen that stored procedure yet...

No, you're never too old to Yak'n'Roll if you're too young to die.

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-17 : 07:02:24

I think your assumption is correct

I haven't shown the stored procedure as I can't use a stored procedure. The one I wrote was a simple insert that set some default values if parameters were empty

-----------

Deja Moo - The feeling you've heard the same bull before.

DonAtWork
Master Smack Fu Yak Hacker

2167 Posts

Posted - 2009-06-17 : 07:40:49

I am lost here...

The simple stored proc is unmaintainable...
The simple code that CALLS said proc is unmaintainable..

W T F are you supposed to write to accomplish this insert then? A fooking prayer?

[Signature]For fast help, follow this link:
http://weblogs.sqlteam.com/brettk/archive/2005/05/25.aspx
Learn SQL or How to sell Used Cars
For ultra basic questions, follow these links.
http://www.sql-tutorial.net/
http://www.firstsql.com/tutor.htm
http://www.w3schools.com/sql/default.asp

elwoos
Master Smack Fu Yak Hacker

2052 Posts

Posted - 2009-06-17 : 08:19:36

Well done Don you got it

Instead of calling a sproc I am supposed to create an INSERT statement in a string and make use of that. I seem to recall seeing some pretty horrible ones around here in the past but can't find any good examples.

So my new (and apparently more maintainable) code will be something like


                Try
                    ' This is the insert statement I have to use
                    ' instead of calling a sproc
                    strSQL = "insert into errors (CallingFunction, Message, MessageID, Source, Status, ExtRefID, CRMRefID, BackOfficeID) values ('" & CallingFunction & "','" & Message & "','" & MessageID & "','" & Source & "','" & Status & "','" & ExtRefID & "','" & CRMRefID & "','" & BackOfficeID & "')"

                    sqlConn = New SqlConnection(dbConnString)
                    sqlConn.Open()

                    cmd = New SqlCommand(strSQL, sqlConn)
                    cmd.CommandType = CommandType.Text
                    cmd.ExecuteScalar()
                    sqlConn.Close()

                    Return something

                Catch ex As Exception
                    ' error occured
                    ' Handle it here
                End Try

I was hoping someone would come up with a much more horrible (but still working) insert string - see StrSQL above.

The one I have put here "works" but it doesn't cope with the case where a delimiter is inside one of the parameters
(think SQL injection, which of course my original code shouldn't have a problem with)

-----------

Deja Moo - The feeling you've heard the same bull before.

jezemine
Master Smack Fu Yak Hacker

2886 Posts

Posted - 2009-06-17 : 10:18:32

it's good that the code above is considered maintainable, because you'll certainly need to do some maintenance when your site gets hacked from sql injection.

webfred
Master Smack Fu Yak Hacker

8781 Posts

Posted - 2009-06-17 : 10:49:10

quote:
Originally posted by jezemine

it's good that the code above is considered maintainable, ...

Always look on the bright side

No, you're never too old to Yak'n'Roll if you're too young to die.

blindman
Master Smack Fu Yak Hacker

2365 Posts

Posted - 2009-06-17 : 11:47:47

Its not the Stored Procedure that is unmaintainable.
Its not the code that is unmaintainable.
It is your job that is unmaintainable.
Instead of rewriting the process, rewrite your resume'.

________________________________________________
If it is not practically useful, then it is practically useless.
________________________________________________

RickD
Slow But Sure Yak Herding Master

3608 Posts

Posted - 2009-06-17 : 12:48:10

quote:
Originally posted by elwoos

Well done Don you got it

Instead of calling a sproc I am supposed to create an INSERT statement in a string and make use of that. I seem to recall seeing some pretty horrible ones around here in the past but can't find any good examples.

So my new (and apparently more maintainable) code will be something like

Try
' This is the insert statement I have to use
' instead of calling a sproc
strSQL = "insert into errors (CallingFunction, Message, MessageID, Source, Status, ExtRefID, CRMRefID, BackOfficeID) values ('" & CallingFunction & "','" & Message & "','" & MessageID & "','" & Source & "','" & Status & "','" & ExtRefID & "','" & CRMRefID & "','" & BackOfficeID & "')"

sqlConn = New SqlConnection(dbConnString)
sqlConn.Open()

cmd = New SqlCommand(strSQL, sqlConn)
cmd.CommandType = CommandType.Text
cmd.ExecuteScalar()
sqlConn.Close()

Return something

Catch ex As Exception
' error occured
' Handle it here
End Try

I was hoping someone would come up with a much more horrible (but still working) insert string - see StrSQL above.

The one I have put here "works" but it doesn't cope with the case where a delimiter is inside one of the parameters
(think SQL injection, which of course my original code shouldn't have a problem with)

-----------

Deja Moo - The feeling you've heard the same bull before.

The easiest way to make this look even worse is to make sure you convert all the variables into their specific type during the insert and use a select instead of Values ()..

jimf
Master Smack Fu Yak Hacker

2875 Posts

Posted - 2009-06-17 : 13:48:57

You can string out the string
strSQL = "insert into "
strSQL = strSQL & "errors"
strSQL = strSQL + " (CallingFunction"
strSQL = strSQL + ","
strSQL = strSQL +" Message"
etc.

This is easily maintainable since you can change just part of the string instead of the whole string!

Jim

blindman
Master Smack Fu Yak Hacker

2365 Posts

Posted - 2009-06-17 : 14:34:18

Magic strings, Jim?
Shame on you for hard-coding like that. Those values should be variable for maximum maintainability:

strInsertInto = "insert into "
strTableName = "error"
strCallingFunction = " (CallingFunction"
strSeparator = ","
strMessage = " Message"

strSQL = strInsertInto
strSQL = strSQL + strTableName
strSQL = strSQL + strCallingFunction
strSQL = strSQL + strSeparator
strSQL = strSQL + strMessage

________________________________________________
If it is not practically useful, then it is practically useless.
________________________________________________

mcrowley
Aged Yak Warrior

771 Posts

Posted - 2009-06-17 : 15:08:34

But you have just doubled the lines, which naturally doubles the complexity. Maybe even triples it!*

Next thing you know, you will want to double space between the lines, causing the complexity to
spiral entirely out of control!

* Hard to say how good elwoos' boss is at math. ;-)

Next Page

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources