Updating the database using parameters in .net

Question

Which one of these two ways would be more recommended to update a database with a given query string:

Option 1:

Dim query As String = "INSERT INTO employee VALUES (@Name, @Age)"
Dim command As New SqlClient.SqlCommand(query, sqlConnection)

Dim params As SqlParameter() = {
    New SqlParameter("@Name", txtName.Value),
    New SqlParameter("@Age", txtAge.Value))
}

Call UpdateDatabase(command, params, NumError, DescError)

Public Sub UpdateDatabase(ByVal command As SqlCommand, ByVal parameters() As SqlParameter, ByRef NumError As Double, ByRef DescError As String)
Try
    For Each parameter In parameters
        command.Parameters.Add(parameter)
    Next
    command.ExecuteNonQuery()
    command.Dispose()
    NumError = 0
    DescError = ""
    Catch ex As Exception
        NumError = Err.Number
        DescError = Err.Description
    End Try
End Sub

Option 2:

Dim query As String = "INSERT INTO employee VALUES (@Name, @Age)"
Dim command As New SqlClient.SqlCommand(query, sqlConnection)

command.Parameters.AddWithValue("@Name", txtName.Value)
command.Parameters.AddWithValue("@Age", txtAge.Value)

Call UpdateDatabase(command, NumError, DescError)

Public Sub UpdateDatabase(ByVal command As SqlCommand, ByRef NumError As Double, ByRef DescError As String)
    Try
        command.ExecuteNonQuery()
        command.Dispose()
        NumError = 0
        DescError = ""
    Catch ex As Exception
        NumError = Err.Number
        DescError = Err.Description
    End Try
End Sub

Or is there any other better way to do this?

Since you never open the connection, neither one is a good way to do the update :) — Tim
– Tim, Commented Sep 5, 2011 at 8:11
I just didn't wanna complicate the code putting more things than necessary there, in this case the sqlConnection object represents the connection wich is already opened. — rfc1484
– rfc1484, Commented Sep 5, 2011 at 8:18

rfc1484 · Accepted Answer · 2011-09-08 10:22:02Z

It looks like you're trying to create a reusable UpdateCommand, which is all well and good. In addtion to not opening the connection, I'm not sure that you're closing the connection (unless the command.Dispose also closes the connection. You'd be better off to move as much of the db connectivity into the Sub as possible.

Also, I'm not sure where you're getting Err. from in your exception block, but I'd recommend designing a better way to handle any exceptions - perhaps log it somewhere.

Finally, make the Sub a Function and return a bool indicating success/failure:

Public Function UpdateDatabase(ByVal sql As String, ByVal parameters() As SqlParameter) As Boolean

    Dim Successful As Boolean = False

    Try
        Using conn As SqlConnection = new SqlConnection(sqlConnection)
            Using command As New SqlCommand(sql, conn)

                command.CommandType = CommandType.Text        

                For Each parameter As SqlParameter In parameters
                    command.Parameters.Add(parameter)
                Next

                conn.Open()

                command.ExecuteNonQuery()
                Successful = True
            End Using       
        End Using        
    Catch ex As Exception
        Successful = False
        ' Do something with the exception
    End Try

End Function

You could then do this:

Dim query As String = "INSERT INTO employee VALUES (@Name, @Age)"

Dim params As SqlParameter() = {
    New SqlParameter("@Name", txtName.Value),
    New SqlParameter("@Age", txtAge.Value))
}

Dim Updated As Boolean = UpdateDatabase(query, params)

This example assumes sqlConnection is a class level variable holding your connection string. You could also read it directly from the config file if desired.

If you don't have any parameters for the command, you'll need to pass in an empty array (or modify the code in the function to check for params = Nothing):

Dim params As SqlParameter()
Dim Updated As Boolean = UpdateDatabase(query, params)

Adam Houldsworth · Accepted Answer · 2011-09-05 08:18:57Z

1

Neither of those are any good for one main reason that has nothing to do with database access...

You are swallowing any and all managed exceptions. Only handle exceptions if you can handle them appropriately, there is hardly ever a need to take an exception and translate it into return values. In my opinion if you do this, it's not longer an exceptional circumstance.

I can't quite remember the VB.NET Using syntax so I can't as yet provide another example. I'd also not use Call, I don't think it's required.

Sort of pseudo-code:

Using Dim command As New SqlCommand("INSERT...", conn)
    command.Parameters.AddWithValue()
    ....

    command.ExecuteNonQuery() // No need to call dispose, Using does that.
End Using

answered Sep 5, 2011 at 8:18

Adam Houldsworth

64.8k11 gold badges161 silver badges190 bronze badges

3 Comments

Tim Over a year ago

+1 for caution against swallowing exceptions. Also, you got the Using syntax mostly correct - I'm pretty sure you don't need to put Dim in there (I could be wrong - I don't write VB code normally).

Adam Houldsworth Over a year ago

@Tim Thanks. Yeah I think the syntax is close enough to make the point about the Using construct. At a previous job, there was a lot of migrated VB to VB.NET code that featured handling error codes (usually with On Error Resume Next or an error: label), so I tend to fall on the side of not trusting it.

Tim Over a year ago

On Error Resume Next...I actually saw that code in some VB.NET (1.1) files I had to work on about a year ago. The first thought that came to mind was "Really?"

Collectives™ on Stack Overflow

Updating the database using parameters in .net

2 Answers 2

Comments

3 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Related