Jump to content
Xtreme .Net Talk

Recommended Posts

Posted

I have written several applications in VS6. I am slowly converting all of them to VS 2003/2005. One conversion issue I am having, is that I have several (20) sequential database updates to do to several tables in . In VB 6 I would just keep using the same connection and then close it after all the updates finish. However, when I try the same thing using VS2003 I have to close then reopen the connection to avoid errors.

 

Am I missing something, Or is this better in the long run. Or, has 2005 fixed the behavior.

Go Beavs!!!
Posted
Are you doing the updates concurrently? It's better to keep the connection open as opposed to opening+closing several times. Connection pooling would be even better.
Posted

well, opening connections are expensive. I've seen this first hand w/ mysql. Here's where it gets conflicting.

It's bad tp keep connections open. you should close them as soon as you're done with them so others can use it. Ofcourse this conflicts with my 1st statement. Connection pooling basically keeps 1 or more connections open. When you close a connection, it returns to the pool but is never actually close.

When you open a pooled connection, you're just retrieving an already open and unused connection from the pool. The .net mssql provider has this enabled by default incase you're using that.

  • *Experts*
Posted

Can we see the code you're using to do actions on the DB? You should be able to open a connection and use it to do multiple actions (INSERTs, UPDATEs, DELETEs, etc.).

 

Are you using ADO.NET to handle transactions, or some other setup (Enterprise Services, the equivalent of COM+ or DTS) or maybe nothing in regards to transactions?

 

-ner

"I want to stand as close to the edge as I can without going over. Out on the edge you see all the kinds of things you can't see from the center." - Kurt Vonnegut
Posted (edited)

A selection of updates. In the real code there are several more

 

        RSSQL = "SELECT Count(proproduction.[process num]) AS [CountOfprocess num] FROM proproduction WHERE (((proproduction.[comp date])='" & date1 & "') AND ((proproduction.active)=-1) AND ((proproduction.[dm no stat])=-1));"
       DMLotRpt = GetRS(RSSQL)(0)
       SQL = "UPDATE [" & Year(date1) & "] SET [" & ForDate(date1) & "] = '" & DMLotRpt & "' WHERE CatID=16;"
       ActionSQL(SQL)
       'oldest date
       RSSQL = "SELECT Min(proproduction.[arrived at icn]) AS [MinOfarrived at icn] FROM proproduction WHERE proproduction.active=-1 AND proproduction.[dm no stat]=-1 AND proproduction.[comp date]='" & date1 & "';"
       SQL = "UPDATE [" & Year(date1) & "] SET [" & ForDate(date1) & "] = '" & FixDate(GetRS(RSSQL)(0)) & "' WHERE CatID=18;"
       ActionSQL(SQL)
       'average
       If DMLotRpt > 0 Then
           RSSQL = "SELECT CAST(AVG(CAST([arrived at icn] AS integer)) AS datetime) AS AvgDateInt FROM         dbo.PROproduction WHERE     (active = - 1) AND ([dm no stat] = - 1) AND ([comp date] = '" & date1 & "')"
           SQL = "UPDATE [" & Year(date1) & "] SET [" & ForDate(date1) & "] = '" & FixDate(GetRS(RSSQL)(0)) & "' WHERE CatID=15;"
           ActionSQL(SQL)
       End If
       'newest
       RSSQL = "SELECT Max(proproduction.[arrived at icn]) AS [MaxOfarrived at icn] FROM proproduction WHERE proproduction.active=-1 AND proproduction.[dm no stat]=-1 AND proproduction.[comp date]='" & date1 & "';"
       SQL = "UPDATE [" & Year(date1) & "] SET [" & ForDate(date1) & "] = '" & FixDate(GetRS(RSSQL)(0)) & "' WHERE CatID=17;"
       ActionSQL(SQL)

   Public Sub ActionSQL(ByVal SQL As String)
       Dim ado As New ADONet
       ado.ActionSQL(SQL, ado.SetCnn(ADONet.CnnStrDosimSQL))
   End Sub

   Public Function GetRS(ByVal SQL) As SqlDataReader
       Dim ado As New ADONet
       Return ado.GetDR(SQL, ado.SetCnn(ADONet.CnnStrDosimSQL), True)
   End Function

ADONet Class

    Public Function SetCnn(ByVal CnnStr As String) As SqlConnection
       SetCnn = New SqlConnection(CnnStr)
       SetCnn.Open()
   End Function

   Public Function GetDR(ByVal SQL As String, ByVal Cnn As SqlConnection, Optional ByVal StartReader As Boolean = False) As SqlDataReader
       If Cnn.State = ConnectionState.Open Then
           Cnn.Close()
           Cnn.Open()
       End If
       Dim Cmd As New SqlCommand(SQL, Cnn)
       Dim DR As SqlDataReader = Cmd.ExecuteReader
       If StartReader = True Then DR.Read()
       Return DR
   End Function

   Public Function GetDA(ByVal Cmd As SqlCommand, ByRef DS As DataSet) As SqlDataAdapter
       GetDA = New SqlDataAdapter
       GetDA.SelectCommand = Cmd
       DS = New DataSet("X")
       GetDA.Fill(DS)
   End Function

   Public Function GetCmd(ByVal SQL As String, ByVal Cnn As SqlConnection) As SqlCommand
       GetCmd = New SqlCommand(SQL, Cnn)
   End Function

   Public Sub ActionSQL(ByVal SQL As String, ByVal Cnn As SqlConnection)
       Dim Cmd As SqlCommand = GetCmd(SQL, Cnn)
       Cmd.CommandText = SQL
       Try
           Cmd.ExecuteNonQuery()
       Catch a As Exception
           MsgBox(a.ToString & vbCr & vbCr & SQL)
           End
       End Try
       Cnn.Close()
   End Sub

Edited by kcwallace
Go Beavs!!!
  • Administrators
Posted

Just a few points to make:

 

Firstly I would strongly recomend against using string concatenation to build SQL statements - they are error prone and can lead to major security holes. Either use stored procedures, or failing that at least use parameterised queries as they are a big improvement over pure stings.

 

Secondly I find the logic in

If Cnn.State = ConnectionState.Open Then
           Cnn.Close()
           Cnn.Open()
       End If
       Dim Cmd As New SqlCommand(SQL, Cnn)
       Dim DR As SqlDataReader = Cmd.ExecuteReader
       If StartReader = True Then DR.Read()
       Return DR

a bit odd in how it closes the connection if it is already open. I personally would open the connection if it wasn't open or just leave it open if it was already open, but close it when I have finished using it rather than leaving it open and then closing and re-opening it when I need it again. Be aware that only a single datareader can be active on a connection at a time, when the reader is finished close it.

I also fail to see the benefit of the StartReader parameter as in every situation I've used a reader the logic has been

While dr.Read()
   'do stuff with dr
End While
dr.Close()

forcing the reader to advance a single record first seems a fairly odd idea, one likely to cause the first record to be skipped or not depending on the parameter.

 

Thirdly I would also consider a more obvious naming convention, SetCnn could just as easily be SetConnection which is far more readable and less likely to cause confusion down the line. Also the name of the GetDA method is not giving the true story, admittedly it does indeed return a DataAdapter; it also trashes the DataSet you pass in as a second parameter replacing it with a brand new one called "X" and populates it with the results of executing the cmd parameter

 

Fourthly some of the methods seem a bit redundant; do SetCnn and GetCmd really simplify the code anymore than just simply creating the command and connection objects directly?

 

You might find some of the information onhttp://msdn.microsoft.com/practices/Topics/patterns/default.aspx?pull=/library/en-us/dnpatterns/html/dp.asp and http://www.microsoft.com/downloads/details.aspx?FamilyId=F63D1F0A-9877-4A7B-88EC-0426B48DF275&displaylang=en worth looking at as these discuss and give decent examples of DataAccess components.

Posting Guidelines FAQ Post Formatting

 

Intellectuals solve problems; geniuses prevent them.

-- Albert Einstein

Posted
I think you'll find if you follow PD's suggestions you'll have an easier time figuring out how to do what you want to in the future without having to ask because it's follows a more logical path and it will allow you to see the overall picture better, not to mention support down the road for the person who didn't write the code will be easier and if you have to revisit it 6 months from now it will be easier (believe it or not) for you too. Critques are a good thing, don't knock them.
  • *Experts*
Posted

A patient says "I seem to run out of breath quite easily and get these headaches all the time. Can you help?"

The doctor says "Well you weight 280 pounds/20 stones/127 kilograms. You should probably follow a better diet and get some excersize."

"Screw that, just tell me how to make my headaches go away."

"Take some aspirin."

 

Good luck with the aspirin, kcwallace.

 

-ner

"I want to stand as close to the edge as I can without going over. Out on the edge you see all the kinds of things you can't see from the center." - Kurt Vonnegut

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...