Custom SQL statement

Question

I am just wondering if my code can still be simplified. I intend to make it reusable in all update statements.

Public Sub updateRecord(ByRef procedure As String, ByRef parameters As String, ByRef obj As String, ByRef LastName As String, ByRef FirstName As String, ByRef MiddleInitial As String, ByRef Age As String, ByRef Address As String)
Dim CS As String = ConfigurationManager.ConnectionStrings("DBCS").ConnectionString
Try
    Using con As New SqlConnection(CS)
        Dim cmd As SqlCommand = New SqlCommand(procedure, con)
        cmd.CommandType = System.Data.CommandType.StoredProcedure
        cmd.Parameters.AddWithValue(parameters, obj)
        cmd.Parameters.AddWithValue("@LastName", LastName)
        cmd.Parameters.AddWithValue("@FirstName", FirstName)
        cmd.Parameters.AddWithValue("@MiddleInitial", MiddleInitial)
        cmd.Parameters.AddWithValue("@Age", Age)
        cmd.Parameters.AddWithValue("@Address", Address)
        cmd.Connection = con
        con.Open()
        Dim TotalRows As Int32 = cmd.ExecuteNonQuery()
    End Using
Catch ex As Exception
    MessageBox.Show(ex.ToString, "Annual Procurement Plan", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
End Try
End Sub

Answer 1

Let us focusing first on the code in question.

Naming

Based on the naming guidelines methods should be named using PascalCase casing and method parameters should be named using camelCase casing.

If you have your own coding/naming guidelines you should stick to them. Right now I don't believe so because you are mixing the casing styles for the input parameter names.

Based on the same guidelines one shouldn't use abbreviations for names. A variable named CS doesn't make it obvious at first glance what it is about, but connectionString would do.

---

Catching Exception isn't really a good way to do error handling. You should catch only the type of exception which can be handled by your code. What I mean with this can be read here

• Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

• If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

• Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.

---

If you want to make this reusable for other update methods, you should pass a Dictionary<TKey,TValue> to the method, where as TKey will be the parameters "name" like @LastName and the TValue will be the values the parameter will/should have like so

Public Sub UpdateRecord(ByRef procedure As String, Dictionary<String, Object> parameters)

You can then add the keys as the parameters name and the values as the value.

Answer 2

• You should wrap the SqlCommand inside of a Using statement as well.

• Also, you don't need to create a variable to execute the command, you aren't even going to return whether or not it ran successfully, so don't create the variable at all, it's extraneous.

• Your indentation for the Sub is off, kind of a big deal with a language, that doesn't have a line termination and is blocked by indentations.

• Surrounding a Using statement with a Try/Catch doesn't look right at all.

• You give the connection to the command twice, you don't need to do that.

• Opening your connection at the last possible second looks weird to me, but shouldn't cause any issues.

• If you have to return the information on an exception, then you should forget about the Using statements and write it all out like this:

---

Public Sub updateRecord(ByRef procedure As String, ByRef parameters As String, ByRef obj As String, ByRef LastName As String, ByRef FirstName As String, ByRef MiddleInitial As String, ByRef Age As String, ByRef Address As String)
    Dim CS As String = ConfigurationManager.ConnectionStrings("DBCS").ConnectionString
    Try
        Dim con As New SqlConnection(CS)
        Dim cmd As SqlCommand = New SqlCommand(procedure, con)
        cmd.CommandType = System.Data.CommandType.StoredProcedure
        cmd.Parameters.AddWithValue(parameters, obj)
        cmd.Parameters.AddWithValue("@LastName", LastName)
        cmd.Parameters.AddWithValue("@FirstName", FirstName)
        cmd.Parameters.AddWithValue("@MiddleInitial", MiddleInitial)
        cmd.Parameters.AddWithValue("@Age", Age)
        cmd.Parameters.AddWithValue("@Address", Address)
        con.Open()
        cmd.ExecuteNonQuery()
    Catch ex As Exception
        MessageBox.Show(ex.ToString, "Annual Procurement Plan", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
    Finally
        cmd.Dispose()
        con.Dispose()
        con.Close
    End Try
End Sub