Reusable code - suggest improvement

[ManicCW]

Hi I have writen code for inserting, updating and deleting records from access database (can be used in sql with little change). I've put this code in module and I call subs as I need them. Here is the code:

This is for inserting (code in module)

Public Sub Inserting(ByVal InsertTable As String, ByVal InsertCols As Array, ByVal InsertParams As Array)
        Dim cmd As New OleDbCommand
        Dim i As Integer
        With cmd
            'insert command text
            .CommandText = "INSERT INTO " & InsertTable & " ("
            For i = 0 To InsertCols.Length - 1 Step 1
                .CommandText &= InsertCols(i)
                If i < InsertCols.Length - 1 Then
                    .CommandText &= ", "
                End If
            Next
            .CommandText &= ") VALUES ("
            For i = 0 To InsertParams.Length - 1 Step 1
                .CommandText &= "?"
                If i < InsertParams.Length - 1 Then
                    .CommandText &= ", "
                End If
            Next
            .CommandText &= ")"
            'parameters
            For i = 0 To InsertParams.Length - 1 Step 1
                .Parameters.Add(InsertCols(i), InsertParams(i))
            Next
            .Connection = cn
        End With
        cmd.ExecuteNonQuery()
    End Sub

This is where I call it in app:

        Dim InsertTable As String = "TimeZones"
        Dim InsertCols(2) As String
        InsertCols(0) = "ZoneName"
        InsertCols(1) = "HourCorrection"
        InsertCols(2) = "MinuteCorrection"
        Dim InsertParams(2) As String
        InsertParams(0) = Me.txtZoneName.Text.Trim
        InsertParams(1) = Me.txtHourCorrection.Text.Trim
        InsertParams(2) = Me.txtMinuteCorrection.Text.Trim

        cn.Open()
        Inserting(InsertTable, InsertCols, InsertParams)
        BindGrid()
        cn.Close()

This works great. BUT, I allways want to do it very simple with less code. Can I use here multidimensional arrays. Suggest me improvements. TNX

 

[jmcilhinney]

Here's my recommendation:

    Public Function Inserting(ByVal table As String, ByVal columnData As Hashtable) As Integer
        Dim cmd As New OleDbCommand
        Dim columns As New System.Text.StringBuilder
        Dim values As New System.Text.StringBuilder

        For Each columnName As String In columnData.Keys
            If columns.Length > 0 Then
                columns.Append(", ")
            End If

            columns.Append(columnName)

            If values.Length > 0 Then
                values.Append(", ")
            End If

            values.Append("?")

            cmd.Parameters.Add(columnName, columnData(columnName))
        Next columnName

        cmd.CommandText = String.Format("INSERT INTO {0} ({1}) VALUES ({2})", _
                                        table, _
                                        columns, _
                                        values)
        cmd.Connection = cn

        Return cmd.ExecuteNonQuery()
    End Function

I take it that "cn" is declared in the module too.

 

[ManicCW]

great, only one for...next loop. tnx

... and if it's not a problem can you write the best way to call your sub from app?

 

[jmcilhinney]

Using the same data as you did previously:

Dim table As String = "TimeZones"
Dim columnData As New Hashtable

columnData.Add("ZoneName", Me.txtZoneName.Text.Trim)
columnData.Add("HourCorrection", Me.txtHourCorrection.Text.Trim)
columnData.Add("MinuteCorrection", Me.txtMinuteCorrection.Text.Trim)
cn.Open()[FONT=monospace]
[/FONT]Inserting(table, columnData)

Note also that I'd be inclined to Open and Close the connection inside the Inserting method, or at least provide a third Boolean argument that indicates whether it should be or not.

 

[jmcilhinney]

Note also that you are passing all three values as String objects, while it would appear from the names that the second and third should actually be numeric.

 

[ManicCW]

yes they are numeric.
so you suggest this:

columnData.Add("HourCorrection", CInt(Me.txtHourCorrection.Text.Trim))

 

[jmcilhinney]

yes they are numeric.
so you suggest this:

columnData.Add("HourCorrection", CInt(Me.txtHourCorrection.Text.Trim))

I don't suggest that unless you already know that the value is valid. If the user enters a non-integer value that code will throw an exception. Have you considered using a NumericUpDown instead of a TextBox. They are often the best option for numeric input, particularly integers.