Good programming - memory allocation
-
Coding Dogs, I would like to know that I am programming correctly & responsibly. Would you say that the new version of this function is correct as it concerns allocating and deallocating new objects? Or do I even have to bother with this? a.k.a. Can I just rely on the .NET environment to handle memory management once a DIM goes out of scope? I am a C# programmer and I feel like the NEW VERSION is correct, but I am not sure. Also, I did not like variables declared on the fly. Thank you in advance for any opinions. William-SD :confused: 'NEW VERSION' Public Shared Function GetData(ByRef SQL As String) As System.Data.DataSet Dim cn As System.Data.SqlClient.SqlConnection Dim adp As System.Data.SqlClient.SqlDataAdapter Dim ds As System.Data.DataSet = New System.Data.DataSet Try cn = New System.Data.SqlClient.SqlConnection(ConnectionString) If Not (cn.State = ConnectionState.Open) Then Throw New System.Exception("Database connection failed") End If adp = New System.Data.SqlClient.SqlDataAdapter(SQL, cn) If (Len(SQL) > 0) Then cn.Open() adp.Fill(ds) GetData = ds Else ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset End If Catch ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset Finally cn.Close() adp.Dispose() cn.Dispose() adp = Nothing cn = Nothing End Try End Function 'OLD VERSION' Public Function GetData(ByRef SQL As String, ByRef ConnectionString As String) As System.Data.DataSet Dim ds As System.Data.DataSet = New System.Data.DataSet If (Len(SQL) > 0) Then Dim cn As System.Data.SqlClient.SqlConnection = Connect(ConnectionString) Try Dim cmd As SqlClient.SqlCommand = cn.CreateCommand() cmd.CommandText = SQL cmd.CommandType = CommandType.Text Dim adp As System.Data.SqlClient.SqlDataAdapter = New SqlClient.SqlDataAdapter(cmd) adp.Fill(ds) Finally cn.Close() End Try End If GetData = ds End Function
-
Coding Dogs, I would like to know that I am programming correctly & responsibly. Would you say that the new version of this function is correct as it concerns allocating and deallocating new objects? Or do I even have to bother with this? a.k.a. Can I just rely on the .NET environment to handle memory management once a DIM goes out of scope? I am a C# programmer and I feel like the NEW VERSION is correct, but I am not sure. Also, I did not like variables declared on the fly. Thank you in advance for any opinions. William-SD :confused: 'NEW VERSION' Public Shared Function GetData(ByRef SQL As String) As System.Data.DataSet Dim cn As System.Data.SqlClient.SqlConnection Dim adp As System.Data.SqlClient.SqlDataAdapter Dim ds As System.Data.DataSet = New System.Data.DataSet Try cn = New System.Data.SqlClient.SqlConnection(ConnectionString) If Not (cn.State = ConnectionState.Open) Then Throw New System.Exception("Database connection failed") End If adp = New System.Data.SqlClient.SqlDataAdapter(SQL, cn) If (Len(SQL) > 0) Then cn.Open() adp.Fill(ds) GetData = ds Else ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset End If Catch ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset Finally cn.Close() adp.Dispose() cn.Dispose() adp = Nothing cn = Nothing End Try End Function 'OLD VERSION' Public Function GetData(ByRef SQL As String, ByRef ConnectionString As String) As System.Data.DataSet Dim ds As System.Data.DataSet = New System.Data.DataSet If (Len(SQL) > 0) Then Dim cn As System.Data.SqlClient.SqlConnection = Connect(ConnectionString) Try Dim cmd As SqlClient.SqlCommand = cn.CreateCommand() cmd.CommandText = SQL cmd.CommandType = CommandType.Text Dim adp As System.Data.SqlClient.SqlDataAdapter = New SqlClient.SqlDataAdapter(cmd) adp.Fill(ds) Finally cn.Close() End Try End If GetData = ds End Function
I'm not sure which way is necessarily better but in both of your finally clauses, if cn fails to be allocated or open for some reason and is nothing then you will get an exception when you do cn.Close(). I've made this same mistake a few times and usually i just fix it like this:
If Not cn Is Nothing Then cn.Close()- Tom -
I'm not sure which way is necessarily better but in both of your finally clauses, if cn fails to be allocated or open for some reason and is nothing then you will get an exception when you do cn.Close(). I've made this same mistake a few times and usually i just fix it like this:
If Not cn Is Nothing Then cn.Close()- TomThe dispose methods does that for you. Simple call cn.Dispose() in your finally.
-
Coding Dogs, I would like to know that I am programming correctly & responsibly. Would you say that the new version of this function is correct as it concerns allocating and deallocating new objects? Or do I even have to bother with this? a.k.a. Can I just rely on the .NET environment to handle memory management once a DIM goes out of scope? I am a C# programmer and I feel like the NEW VERSION is correct, but I am not sure. Also, I did not like variables declared on the fly. Thank you in advance for any opinions. William-SD :confused: 'NEW VERSION' Public Shared Function GetData(ByRef SQL As String) As System.Data.DataSet Dim cn As System.Data.SqlClient.SqlConnection Dim adp As System.Data.SqlClient.SqlDataAdapter Dim ds As System.Data.DataSet = New System.Data.DataSet Try cn = New System.Data.SqlClient.SqlConnection(ConnectionString) If Not (cn.State = ConnectionState.Open) Then Throw New System.Exception("Database connection failed") End If adp = New System.Data.SqlClient.SqlDataAdapter(SQL, cn) If (Len(SQL) > 0) Then cn.Open() adp.Fill(ds) GetData = ds Else ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset End If Catch ds.Dispose() ds = Nothing GetData = New System.Data.DataSet 'return nothing in an empty dataset Finally cn.Close() adp.Dispose() cn.Dispose() adp = Nothing cn = Nothing End Try End Function 'OLD VERSION' Public Function GetData(ByRef SQL As String, ByRef ConnectionString As String) As System.Data.DataSet Dim ds As System.Data.DataSet = New System.Data.DataSet If (Len(SQL) > 0) Then Dim cn As System.Data.SqlClient.SqlConnection = Connect(ConnectionString) Try Dim cmd As SqlClient.SqlCommand = cn.CreateCommand() cmd.CommandText = SQL cmd.CommandType = CommandType.Text Dim adp As System.Data.SqlClient.SqlDataAdapter = New SqlClient.SqlDataAdapter(cmd) adp.Fill(ds) Finally cn.Close() End Try End If GetData = ds End Function
I would write it like this: If you need an explaination on any reason why I did certain things, just let me know. Public Shared Function GetData(ByRef SQL As String) As System.Data.DataSet Dim cn As New System.Data.SqlClient.SqlConnection(ConnectionString) Dim adp As New System.Data.SqlClient.SqlDataAdapter(SQL, cn) Dim ds As New System.Data.DataSet Try If SQL.Length > 0 Then cn.Open() adp.Fill(ds) End If Finally adp.Dispose() cn.Dispose() End Try Return ds End Function
-
The dispose methods does that for you. Simple call cn.Dispose() in your finally.
-
I would write it like this: If you need an explaination on any reason why I did certain things, just let me know. Public Shared Function GetData(ByRef SQL As String) As System.Data.DataSet Dim cn As New System.Data.SqlClient.SqlConnection(ConnectionString) Dim adp As New System.Data.SqlClient.SqlDataAdapter(SQL, cn) Dim ds As New System.Data.DataSet Try If SQL.Length > 0 Then cn.Open() adp.Fill(ds) End If Finally adp.Dispose() cn.Dispose() End Try Return ds End Function
Does Dispose() work if the variable is never initialized? Example: Dim MyObject As Object MyObject.Dispose() I suppose I could test it quick but does that throw an exception because of a null object reference? Edit: Ok tested with following code Dim oObject As New Object oObject = Nothing oObject.Dispose() Which threw a NullReferenceException. This is why I was wondering if it was safe to just put cn.Dispose() in the finally clause instead of enclosing it in If Not cn Is Nothing Then block. That way you are covered if for some reason it never got initialized. Unless I'm missing something.
-
Does Dispose() work if the variable is never initialized? Example: Dim MyObject As Object MyObject.Dispose() I suppose I could test it quick but does that throw an exception because of a null object reference? Edit: Ok tested with following code Dim oObject As New Object oObject = Nothing oObject.Dispose() Which threw a NullReferenceException. This is why I was wondering if it was safe to just put cn.Dispose() in the finally clause instead of enclosing it in If Not cn Is Nothing Then block. That way you are covered if for some reason it never got initialized. Unless I'm missing something.
Don't worry about the
whatever = Nothing. Just call the.Dispose()method on it. As soon as the object goes out of scope, the reference is automatically dropped and the GC collects it, when it gets around to it. RageInTheMachine9532 "...a pungent, ghastly, stinky piece of cheese!" -- The Roaming Gnome -
Don't worry about the
whatever = Nothing. Just call the.Dispose()method on it. As soon as the object goes out of scope, the reference is automatically dropped and the GC collects it, when it gets around to it. RageInTheMachine9532 "...a pungent, ghastly, stinky piece of cheese!" -- The Roaming GnomeI realize you don't have to set an object reference to nothing. That wasn't my point. The point was you can't call .Dispose() on an NullReference. So if for some reason the object never gets assigned a reference (say there is a memory allocation problem for some reason) then in the finally clause the program would crash if there is a object.dispose() statement.
-
I realize you don't have to set an object reference to nothing. That wasn't my point. The point was you can't call .Dispose() on an NullReference. So if for some reason the object never gets assigned a reference (say there is a memory allocation problem for some reason) then in the finally clause the program would crash if there is a object.dispose() statement.
There is no NullReference Exception in my example, I declare the variables as new at the top of the sub. If there's a memory allocation problem at some point that prevents the variable from being initialized, then the user has way more problems than your app crashing :)