Well coded or not?
-
I have the following code. I am returning a string. If I use return where I have in this way is there a chance I could have left some database connections open still? Does it skip the connection "Finish with resources" section? I believe this is the cause to alot of timeout exceptions. Any help much appreciated.
'Gets any previosuly create about us text Public Function GetAboutUs(ByVal ProductID As Int32) As String ' Create Connection Dim HSConn As New SqlConnection(ConfigurationManager.ConnectionStrings("RemoteSqlServer").ConnectionString) ' Create data reader Dim dr As SqlDataReader 'Declare Command Dim HSComm As New SqlCommand() Try HSConn.Open() ' Provide connection HSComm.Connection = HSConn ' Provide command type and command HSComm.CommandType = CommandType.StoredProcedure HSComm.Parameters.AddWithValue("ProductID", ProductID) HSComm.CommandText = "SP_ProductAboutUsSelect" 'Execute the command dr = HSComm.ExecuteReader While dr.Read() 'If there is a Tagline return it. Otherwise return nothing If dr.IsDBNull(0) Then Return Nothing Else Return dr.GetString(0) End If End While 'Return in case value is not found Return Nothing Catch ex As Exception ' Failed to retrieve tagline so return false Emailer.ErrorEmail(ex, Page) Return Nothing End Try ' Finish up with resources dr.Close() HSConn.Close() End Function
-
I have the following code. I am returning a string. If I use return where I have in this way is there a chance I could have left some database connections open still? Does it skip the connection "Finish with resources" section? I believe this is the cause to alot of timeout exceptions. Any help much appreciated.
'Gets any previosuly create about us text Public Function GetAboutUs(ByVal ProductID As Int32) As String ' Create Connection Dim HSConn As New SqlConnection(ConfigurationManager.ConnectionStrings("RemoteSqlServer").ConnectionString) ' Create data reader Dim dr As SqlDataReader 'Declare Command Dim HSComm As New SqlCommand() Try HSConn.Open() ' Provide connection HSComm.Connection = HSConn ' Provide command type and command HSComm.CommandType = CommandType.StoredProcedure HSComm.Parameters.AddWithValue("ProductID", ProductID) HSComm.CommandText = "SP_ProductAboutUsSelect" 'Execute the command dr = HSComm.ExecuteReader While dr.Read() 'If there is a Tagline return it. Otherwise return nothing If dr.IsDBNull(0) Then Return Nothing Else Return dr.GetString(0) End If End While 'Return in case value is not found Return Nothing Catch ex As Exception ' Failed to retrieve tagline so return false Emailer.ErrorEmail(ex, Page) Return Nothing End Try ' Finish up with resources dr.Close() HSConn.Close() End Function
Hi, this does not look OK; everytime you return, the remainder of the function is not executed, in particular the Close statements at the end. There are two solutions to this: 1. don't use return except as the last statement; instead store the result value in some variable (say result), and proceed throughout the function, that ends on return result 2. use a try/catch/finally construct with the Close lines in the finally block; they will get executed even when you exit the try or catch parts in whatever way (even with return). :)
Luc Pattyn [Forum Guidelines] [My Articles]
this months tips: - use PRE tags to preserve formatting when showing multi-line code snippets - before you ask a question here, search CodeProject, then Google
-
I have the following code. I am returning a string. If I use return where I have in this way is there a chance I could have left some database connections open still? Does it skip the connection "Finish with resources" section? I believe this is the cause to alot of timeout exceptions. Any help much appreciated.
'Gets any previosuly create about us text Public Function GetAboutUs(ByVal ProductID As Int32) As String ' Create Connection Dim HSConn As New SqlConnection(ConfigurationManager.ConnectionStrings("RemoteSqlServer").ConnectionString) ' Create data reader Dim dr As SqlDataReader 'Declare Command Dim HSComm As New SqlCommand() Try HSConn.Open() ' Provide connection HSComm.Connection = HSConn ' Provide command type and command HSComm.CommandType = CommandType.StoredProcedure HSComm.Parameters.AddWithValue("ProductID", ProductID) HSComm.CommandText = "SP_ProductAboutUsSelect" 'Execute the command dr = HSComm.ExecuteReader While dr.Read() 'If there is a Tagline return it. Otherwise return nothing If dr.IsDBNull(0) Then Return Nothing Else Return dr.GetString(0) End If End While 'Return in case value is not found Return Nothing Catch ex As Exception ' Failed to retrieve tagline so return false Emailer.ErrorEmail(ex, Page) Return Nothing End Try ' Finish up with resources dr.Close() HSConn.Close() End Function
No. It gave me a headache just looking at it ;P You should rewrite it a bit.
"Any sort of work in VB6 is bound to provide several WTF moments." - Christian Graus