possible optimization?

[Anti-Rich]

hi all,

i have the following sub that executes an update/delete/insert statement on my sqlserver, i was just wondering if this is a "good programmers" (ie. most efficient) way of doing it.

PublicSub NonQuery(ByVal sql AsString, ByRef cnn As SqlConnection, ByVal fields() AsString, ByVal values() AsString)
 
cmd = New SqlCommand
With cmd
.CommandType = CommandType.Text
.CommandText = sql
.Connection = cnn
For i AsInteger = 0 To fields.Length - 1
.Parameters.AddWithValue("@" & fields(i), values(i))
Next
Try
.Connection.Open()
.ExecuteNonQuery()
.Connection.Close()
Catch ex As Exception
MsgBox(ex.ToString)
Finally
If .Connection.State = ConnectionState.Open Then
.Connection.Close()
EndIf
EndTry
EndWith
 
End Sub

if your wondering the reason i used an array for the fields and values its because i wanted a generic sub that can do it with the least fuss, but at the same time handling a dynamic amount of fields/values. im hoping this was a good way to do it.

any input/suggestions/improvements would be much appreciated

regards
adam

 

[cjard]

well, it kinda removes the point of having parameterized queries with strongly-typed parameters

What you have gained in code-shortcutting you possibly sacrifice in functionality, type strength and speed.

One possible improvement would be to pass your values in as Object instead of string. That way you can still pass a Date type object in and have the code understand that it's to be a date rather than having to format your whole database as string, or cross your fingers and hope that an implicit conversion from string will work (i strongly advise you avoid implicit conversions)

Additionally my recommendations would be:

Store your commands in a Dictionary once you have made them. When requesting to make another, first query the dictionary for its existence. Give each statement a unique friendly name in code, and use it for the key

Additionally, dont can the running code to simply return errors in a message box; there may be cases where you wish to be specific. Provide a method that builds and stores commands in a dictionary and another method that runs them in canned mode. Your other modules should then:

Get a statement (it is either made or retrieved from the dictionary)
Run it itself and deal with the errors specially
OR
Pass it to the canned run-and-messagebox-the-errors method


Dont use MsgBox - it's a cling to the legacy of VB6 and reduces your ability to read and interoperate with other .net syntaxes. For a complete list of the legacy functions that you are using, remove the reference to Microsoft.VisualBasic from your project properties. Every compiler error generated as a result is a legacy compatibility function

 

[Anti-Rich]

hey cjard

thanks for your input, i never thought of passing values in as an object, so much more flexibility! god bless forums! just curious though, how do you use a dictionary? im assuming the dictionary is a replacement for my string array of sql statements, yes? ive never worked with dictionaries and barely ever even heard of them

oh and the msgbox thing was just how my lecturer taught me, is there another better way than the msgbox?

your feedback would be much appreciated

regards
adam

 

[cjard]

hey cjard

thanks for your input, i never thought of passing values in as an object, so much more flexibility! god bless forums! just curious though, how do you use a dictionary?

Dim dic as New System.Collections.Generic.Dictionary(Of String, SqlCommand)

dic("myInsertCommand") = New SqlCommand("INSERT ...")
dic("myUpdateCommand") = New SqlCommand("UPDATE ...")

The idea of a dictionary is of relating one object with another. its like an array of X where a number is associated with x:

Dim myArray(0 to 9) of String
myArray(0) = "hello"

the number 0 is associated with "hello"


in the dictionary, we specify what we are going to be using for the key, and what type of value:
Dictionary(Of String, SqlCommand)

Essentially here we are going to be stringing the commands


-
So, in your case when you make a command, add it to the dictionary with a key of the statement.. and when someone comes and tries to use a statement , first heck to see if its in the dict.. change the parameters accordingly and run it

Actually, the parameters collection is a dictionary.. it associates Parameter objects with a string - the parameter name

im assuming the dictionary is a replacement for my string array of sql statements, yes? ive never worked with dictionaries and barely ever even heard of them

Kind of.. they are like a flexible, named-index array. Very useful, and you may never go back to arrays once you start using List and Dictionary

oh and the msgbox thing was just how my lecturer taught me, is there another better way than the msgbox?

The .NET way is MessageBox.Show() and it works across all .net syntaxes.

As noted before, in project properties, you have the References tab, unticking Microsoft.VisualBasic will highlight any legacy, non-.NET ways you are coding..

 

[Anti-Rich]

hi cjard,

although i understand the benefits of what you are suggesting, i have a couple of issues..

ok the first one being, if i specify the commandtext in the sqlcommand going into the dictionary, dont i lose the flexibility of being able to dynamically change the commandtext... now i do understand i can change it using the sqlcommand.commandtext property... but if im going to change it later on (and odds are i probably am) whats the point of putting it into the dictionary? and in regards to the parameters, the reason i did it the way that i did is so i have a generic command than can handle any amount of parameters, for any sort of nonquery statement.

so, in summary, whats the point of specifying the commandtext in the command (when adding it to the dictionary), when im always going to change it and its always going to have different parameters? i may as well just create/re-instanciate the command when i need to use it...

maybe im getting things a bit twisted, im not sure, but thank you so much for your input so far!! if you could answer that question id be greatly appreciative

regards
adam

 

[cjard]

i find it hard to believe that anyone would write a database aware app where, during the life of the app, no two queries would be run that are the same (variables don't count). If this is the case for your app, then by all means, ignore the dictionary advice ..

 

[Anti-Rich]

sorry cjard, i dont mean to be a pain and i greatly appreciate your advice so far. im not sure about my queries, but throughout my project now at least ill be able to look out for any consistencies with my queries (ive never really had to take notice of it before now) and apply the dictionary advice when its needed (like, for example, if i have an array of sql statements, ill just use a List or Dictionary instead). so thankyou very much for all your advice, i know it will be invaluable down the track

regards
adam

 

[cjard]

Most DB apps I've ever written have had at least some consistent queries for selected, updating, inserting and deleting data. These are the queries that should be retained

Additionally, one of the main aims of preparing a statement with parameters is that the client side can build it once, the server side can compile and lazily evaluate it for cost and over time choose the best plan. It is likely that the major databases will look at submitted straight text queries anyway and find ones they deem common (and schedule a thorough cost analysis when idle) so you could see preparing a statement as a hint that the DB may do this. Other DBs may only cache and work on prepared statements.

In pseudocode the notion is that we would do this:

'build the parameterised command
'add the parameter types
'set the values of the parameters
'run the query

'set different values of the parameters
'run the query

'set more values
'run the query

...

The idea is that we dont spend CPU cycles recreating what was once created, and the database knows which queries are likely to be re-used

Your method would work something like:

Function GetDataTableFromQuery(friendlyName as String, commandText as String, paramNames() as String, paramVals() as Object) As DataTable
 
Dim cmd as SomeDbCommand
if(commandsDict.ContainsKey(friendlyName)) then
  cmd = commandsDict(friendlyName)
else
  cmd = New SomeDBCommand(comamndText)
  commandsDict.Add(friendlyName, cmd)
  foreach s as string in paramnames
    'add the parameters etc DONT set the values
  next
endif
 
for i as integer = 0 to paramnames.Length-1
  cmd.Parameters(paramnames(i)).Value = paramvalues(i)
next i
 
'run the query now and return results
End Function

THis way you can say:

DIm pn as New String(){"ID", "country"}
Dim pv as New String(){"1234356", "united states"}
Dim x as Datatable = DBModule.GetDataTableFromQuery("Form1_mainquery", "SELECT Name, Age FROM Person WHERE ID = @ID AND Country = @country", pn, pv)


and to rerun the query for another ID also still in the USA, you just need:
pn as New String(){"ID"}
pv as New String(){"987654"}

x = DBModule.GetDataTableFromQuery("Form1_mainquery", Nothing, pn, pv)

 

[Anti-Rich]

ok, i actually understand what your saying!

another question though, i noticed you used a datatable... ive never used those before, what are the benefits/uses of a datatable? are they similar to the dataset?


cheers
adam

 

[cjard]

i noticed you used a datatable... ive never used those before

If you've downlaoded data from a database and intoa .NET app, you've used a DataTable, trust me..

DataSets dont hold data, they hold DataTables and DataTables hold data.

If you say:

myTA.Fill(myDataSet.Tables("myDataTable"))
myTA.Fill(myDataSet, "myDataTable")
myTA.Fill(myDataSet.myDataTable)

youre using a datatable each time. Think about it
Noone has ever successfully downloaded data into a DataSet that contains no DataTables

 

[Anti-Rich]

haha, yeah that was pretty stupid of me now that i think of it

oh and i implemented a dictionary (of sqlcommands) in another project im now working on and it works perfectly! thanks heaps for all your help!

regards
adam