0

Please have a look at the code below:

'Form1.vb
Imports System.Data.SqlClient

Public Class Form1

    Private Sub Form1_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load
        'ExecuteDataReader(Function(x) New Person With {.URN = x("URN")})
        Try
            Dim results As IEnumerable(Of Person) = ExecuteDataReader(Function(x) New Person With {.URN = x("URN")})
            For Each c As Person In results 'Line 4
            Next
        Catch ex As Exception

        Finally

        End Try

    End Sub

    Public Function ExecuteDataReader(ByVal castRow As Func(Of IDataRecord, Person)) As IEnumerable(Of Person)
        Try
            Dim objCon As New SqlConnection("Data Source=IANSCOMPUTER;Initial Catalog=Test;Integrated Security=True")
            Dim objCommand As New SqlCommand
            Dim objDR As SqlDataReader
            objCon.Open()
            objCommand.Connection = objCon
            objCommand.CommandText = "SELECT URN FROM Person"
            objDR = objCommand.ExecuteReader()
            Do While objDR.Read
                castRow(objDR)
            Loop
        Catch ex As Exception

        End Try

    End Function
End Class

'Person.vb
Public Class Person
    'Implements IEnumerator, IEnumerable
    Public URN As String
End Class

Why is the results variable empty on line 4. I am new to IEnumberators. The version of .NET I am using (3.5) does not allow for the Yield keyword.

Update Damien_The_Unbeliever has corrected the code. Do you think this pattern is suitable for a Data Logic Layer. I believe I have four options:

1) Return Data Tables instead of Data Readers to the Business Logic Layer. I am then able to wrap the code in Using statements.
2) Return Data Readers to the Business Logic Layer using the pattern described in Damien_The_Unbeliever's answer (not wrapping the disposable objects in Using statements).
3) Return Data Readers to the Business Object Layer and only closing the connection when the DataReader is closed i.e. dr = cmd.ExecuteReader(CommandBehavior.CloseConnection)
4)Don't have a Data Access Layer. Open and close connections in the Business Logic Layer as and when they are required. I believe this makes the code less maintainable.

If there is another option then please let me know.

w0051977
  • 15,099
  • 32
  • 152
  • 329
  • Your function doesn't return anything. – SLaks Nov 18 '12 at 13:24
  • You should get a compiler warning for this code. Don’t ignore it. – Konrad Rudolph Nov 18 '12 at 13:28
  • @SLaks, Thanks. I am trying to adapt the code in an answer to a previous question I wrote here: http://stackoverflow.com/questions/13421695/datareader-outlive-connection-object – w0051977 Nov 18 '12 at 13:30
  • You don't do anything with the result of `castRow`... – Damien_The_Unbeliever Nov 18 '12 at 13:33
  • @Damien_The_Unbeliever,thanks. I am trying to adapt the code in the accepted answer of this question: http://stackoverflow.com/questions/13421695/datareader-outlive-connection-object, for .NET 3.5. Can you tell me how to populate the Results variable with Persons? – w0051977 Nov 18 '12 at 13:37

1 Answers1

2

Try this instead:

Public Function ExecuteDataReader(ByVal castRow As Func(Of IDataRecord, Person)) As IEnumerable(Of Person)
    Using objCon As New SqlConnection("Data Source=IANSCOMPUTER;Initial Catalog=Test;Integrated Security=True")
      Using objCommand as New SqlCommand("SELECT URN FROM Person",objCon)
        Dim objDR As SqlDataReader
        objCon.Open()
        objDR = objCommand.ExecuteReader()
        Dim ret as New List(Of Person)
        Do While objDR.Read
            ret.Add(castRow(objDR))
        Loop
        Return ret
      End Using
    End Using
End Function

Which a) Removes the bad example of "error handling" which silently swallows errors, b) Wraps the SqlCommand and SqlConnection objects so that they get properly disposed, and c) Actually returns something from the function, which was where you were going wrong.

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • Thanks. With this solution, Person does not have to implement IEnumerable or IEnumerator. Is this because List implements these interfaces and you are returning a List? – w0051977 Nov 18 '12 at 13:48
  • Yes, `List(Of T)` implements `IEnumerable(Of T)`. – Damien_The_Unbeliever Nov 18 '12 at 14:13
  • Thanks +1 for the code. I have updated the question. Could you have a look at the update to my question and let me know what option you believe is best? I will then mark the question answered. Thanks again. – w0051977 Nov 18 '12 at 14:34
  • @w0051977 - re: your update - unless you love writing lots of repetitive code (which is still easy to get wrong, leak connections as in your original, etc), I'd advise looking into an ORM (or micro-ORM) and not write any DB code yourself. – Damien_The_Unbeliever Nov 18 '12 at 15:31
  • ,thanks. Your last comment is my longer term plan. My short term plan will be one of those options in my update. Do you have an opinion, which is most suitable? – w0051977 Nov 18 '12 at 17:44
  • @w0051977 - what mobile phone plan is most suitable? What Internet access plan is most suitable? There's no way to know the answers to such questions without knowing specifics (and this probably isn't the best place to work it out) – Damien_The_Unbeliever Nov 18 '12 at 17:46