Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am new to vb.net and one of the functions I wrote is below. The idea behind it is to just send a dictionary and table name to the function and get the result i din return.

Is this good practice or just silly?

   Public Function dbInsert(ByRef EntryDetails As Dictionary(Of String, String),ByRef table as String) As Integer
        Dim command As new SqlCeCommand

        ' Loop over entries.
        Dim fieldNames() As String = {}
        Dim fieldsValues() As String = {}
        Dim pair As KeyValuePair(Of String, String)
        For Each pair In EntryDetails
            ReDim Preserve fieldNames(0 To UBound(fieldNames) + 1)
            fieldNames(UBound(fieldNames)) = pair.Key
            command.Parameters.AddWithValue("@" + pair.Key, pair.Value)
        Next

        command = New SqlCeCommand("INSERT INTO "+table+" (" + String.Join(",", fieldNames) + ")", connection.DbPublic)

        Dim newProdID As Int32 = 0
        Try
            command.ExecuteNonQuery()

            Dim cmd As New SqlCeCommand("SELECT @@IDENTITY", conn)
            newProdID = Convert.ToInt32(cmd.ExecuteScalar())
            'Console.WriteLine("added: {0}", newProdID)
            Return newProdID
        Catch e As Exception
            Console.WriteLine("Unable to insert: {0}", e.Message)
            Return 0
        End Try
    End Function
share|improve this question
2  
Using byref parameters when not needed is bad practice. –  Guffa Mar 6 '12 at 17:09
add comment

1 Answer

This approach is not much good, but may be suit your required functionality. The method name is dbInsert and you are passing table and data in dictionary. you can make it much better by using better naming convention and parameter types.

On the place of this Create a method with following parameter as: `GetIdentityOnInsert(string sqlQuery, CommandType commandType, SqlParameterCollection sqlParameters)emphasized text

query will be: query = "Insert into tableName value(@param1,@param2);SELECT @@IDENTITY;"

///

Public Function GetIdentityOnInsert(ByVal sqlQuery as String, ByVal commandType as Commandtype, ByVal sqlParameters as SqlParameterCollection)            
Try
Using connection As New SqlConnection(connectionString)

        ' Create the command and set its properties.
        Dim command As SqlCommand = New SqlCommand()
        command.Connection = connection
        command.CommandText = sqlQuery
        command.CommandType = commandType
        Dim parameter As New SqlParameter()

        For Each parameter In sqlParameters            
            command.Parameters.Add(parameter);
        Next      

        ' Open the connection and execute the reader.
        connection.Open()
        Dim newProdID As Int32 = 0
        ' try ExecuteScalar() if it gives error then replace it to ExecuteNonQuery()
        ' and replace the below code as in your question.
        newProdID = command.ExecuteScalar() 

            'Dim cmd As New SqlCeCommand("SELECT @@IDENTITY", conn)
            'newProdID = Convert.ToInt32(cmd.ExecuteScalar())
            'Console.WriteLine("added: {0}", newProdID)
         Return newProdID

End Using
Catch e As Exception
    Console.WriteLine("Unable to insert: {0}", e.Message)
    Return 0
End Try
End Function

Hope this help you to create you Data Access Layer. you can extend ExecuteNonQuery with the above method.. just you have to customize little bit..

share|improve this answer
 
that functionality doesn't work on Compact Edition 3.5 –  renevdkooi Mar 12 '12 at 4:47
 
why not.. it works same as i explored through these links: link1, link2, msdn link. this should work well.. –  Niranjan Kala Mar 12 '12 at 7:19
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.