Why not use static/shared methods?

  • Thread starter Thread starter Guest
  • Start date Start date
G

Guest

When I implement my DB classes/DAL layer, why shouldn't I make the methods
(in VB) 'shared'?
Forget the other questions I have right now about whether to put in that
finally block, etc. Why not make the function shared? Why bother have the
class be instatiated and then the method invoked- this method could be
shared, it really has no state, right?
-Dan R


For example, let's say I've got the following method of the DAL class
(you've seen something like this)

Public Function GetSqlDataReader(ByVal strSQL As String) As SqlDataReader
Dim con As SqlConnection = New SqlConnection(DBAccess.SQL_CONN_STR)
Dim cmd As SqlCommand = New SqlCommand(strSQL, con)
Dim dr As SqlDataReader
Try
con.Open()
dr = cmd.ExecuteReader(CommandBehavior.CloseConnection) 'We do
this so that a .close on the SqlDataReader will close the connection.
Return dr
Catch ex As Exception
Throw ex 'Rethrows it. I don't need to put the ex, I don't
think, incidentally
Finally 'From
http://www.dotnet247.com/247reference/msgs/8/40569.aspx is this overkill?
If (Not dr Is Nothing) Then 'It was left open
dr.Close()
End If
End Try
End Function
 
(inline)
When I implement my DB classes/DAL layer, why shouldn't I make the methods
(in VB) 'shared'?

The only reason would be if you wanted to use transactions, or otherwise
needed to guarantee that a number of data operations performed by the same
thread occured on the same connection.
Forget the other questions I have right now about whether to put in that
finally block, etc. Why not make the function shared? Why bother have the
class be instatiated and then the method invoked- this method could be
shared, it really has no state, right?
-Dan R


For example, let's say I've got the following method of the DAL class
(you've seen something like this)

Public Function GetSqlDataReader(ByVal strSQL As String) As SqlDataReader
Dim con As SqlConnection = New SqlConnection(DBAccess.SQL_CONN_STR)
Dim cmd As SqlCommand = New SqlCommand(strSQL, con)
Dim dr As SqlDataReader
Try
con.Open()
dr = cmd.ExecuteReader(CommandBehavior.CloseConnection) 'We do
this so that a .close on the SqlDataReader will close the connection.
Return dr

Catch ex As Exception
Throw ex

The only reason to have such a bare catch block is to have somewhere to set
a breakpoint. It behaves identically to not having a catch block.

think, incidentally
Finally 'From
http://www.dotnet247.com/247reference/msgs/8/40569.aspx is this overkill?
If (Not dr Is Nothing) Then 'It was left open
dr.Close()

This is just wrong. You are passing the datareader out of this function,
and so you must leave it open and trust your client to close it. The
finally block runs just after the return statement, but before the thread
actually returns to the calling function. The datareader would be closed by
the time the client got it. If you can't trust your client to close the
datareader, return a DataTable instead.
End If
End Try
End Function

David
 
About the finally block... Good, I'm glad someone had a comment! :)

I figured it was better safe than sorry- but it's an interesting design
question, isn't it? I was thinking the finally block could be used to make
sure to clean up after the person who called me. That after they were done,
_then_ the finally block would get called.

Are you sure about that thread thing? I thought that the finally block would
be called after the calling function exited- that I've sort of added my
cleanup code so that I can clean up after the caller. The catch/throw was
there to just rethrow any errors.

Regards,
-Dan

dr.Close()

This is just wrong. You are passing the datareader out of this function,
and so you must leave it open and trust your client to close it. The
finally block runs just after the return statement, but before the thread
actually returns to the calling function. The datareader would be closed by
the time the client got it. If you can't trust your client to close the
datareader, return a DataTable instead.
 
But this is ok, right, because this stuff is being returned by value? And
should I put my return after the finally, then, for clarity?
Thanks!
-Dan

Public Shared Function GetDataTable(ByVal strSQL As String) As DataTable
Dim con As SqlConnection = New SqlConnection(DBAccess.SQL_CONN_STR)
Dim sqlDA As SqlDataAdapter
Dim resultDS As DataSet
Dim retTable As DataTable 'This is what we will be handing back.
Try
con.Open()
sqlDA = New SqlDataAdapter ' Create a new SQLDataAdapter object
resultDS = New DataSet ' Create a new DataSet object
sqlDA.SelectCommand = New SqlCommand(strSQL, con) ' Add a
SelectCommand object
sqlDA.SelectCommand.CommandType = CommandType.Text 'Specify the
Select Command type; Just some SQL
sqlDA.Fill(resultDS, "tmpTblName") ' Populate the DataSet with the
returned data. Make up a table name.
retTable = resultDS.Tables("tmpTblName") ' Retrieve the table from
the DataSet.
Catch
Throw
Finally
sqlDA.Dispose()
con.Close()
End Try
Return retTable
End Function
 
But this is ok, right, because this stuff is being returned by value?
.. . .
Public Shared Function GetDataTable(ByVal strSQL As String) As DataTable

Sort of, here you are returning a reference to a DataTable object.
That's fine because the DataTable is a disconnected object.
You can close the connection as soon as you fill the DataTable.
And
should I put my return after the finally, then, for clarity?

A matter of taste, I usually "return" from inside the try block.

David
 
Back
Top