0

I was working doing a lot of refactoring in a legacy code, and I found this:

Public Function GetDocumentTypes() As DataSet
        Dim ds As DataSet = Nothing
        Try
            ds = SqlHelper.ExecuteDataset(Conn, "USP_DocumentTypes_Get")
        Catch ex As Exception
            ErrorHandler.Publish(ex, ExceptionDestinationEmail, TSecurity.GetUserID)
        Finally
            If ds IsNot Nothing Then
                ds.Dispose()
            End If
        End Try
        Return ds
    End Function

And I started doing a research about how the try catch finally flow works, and I realize that in this case always the ds will be nothing because of the finally statement, then I changed the code to:

Public Function GetDocumentTypes() As DataSet
        Dim ds As DataSet = Nothing
        Try
            ds = SqlHelper.ExecuteDataset(Conn, "USP_DocumentTypes_Get")
            Return ds
        Catch ex As Exception
            ErrorHandler.Publish(ex, ExceptionDestinationEmail, TSecurity.GetUserID)
        Finally
            If ds IsNot Nothing Then
                ds.Dispose()
            End If
        End Try
    End Function

Then started receiving a compiler warning: Function doesn't return a value on all code paths.

At the end I solved the issue writing the code like this:

Public Function GetDocumentTypes() As DataSet
        Dim ds As DataSet = Nothing
        Try
            ds = SqlHelper.ExecuteDataset(Conn, "USP_DocumentTypes_Get")
            Return ds
        Catch ex As Exception
            ErrorHandler.Publish(ex, ExceptionDestinationEmail, TSecurity.GetUserID)
            Return ds
        Finally
            If ds IsNot Nothing Then
                ds.Dispose()
            End If
        End Try
    End Function

is that the best way to do it??

ddieppa
  • 5,866
  • 7
  • 30
  • 40

2 Answers2

0

Yes, that seems like a reasonable approach.

RouteMapper
  • 2,484
  • 1
  • 26
  • 45
0

It's fine, just when you call that function do something like this so you know your dataset has a value and did not went in the catch statement:

if Not GetDocumentTypes is nothing then
'your code here
end if
bto.rdz
  • 6,636
  • 4
  • 35
  • 52