Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm working on a small project with dBase files from a 3rd party. Realizing that most of the code to fetch tables and form objects is the same, I made this function:

public static IEnumerable<T> OleDbFetch<T>(Func<OleDbDataReader, T> formator, string connectionString, string query)
{
    var conn = new OleDbConnection(connectionString);
    conn.Open();
    var cmd = new OleDbCommand(query, conn);
    var reader = cmd.ExecuteReader();
    while (reader.Read())
        yield return formator(reader);
    conn.Close();
}

That way I can use it with all sorts of classes (pre-made, imposed) that have mostly parameter-less constructors:

class Person
{
    public int ID { get; set; }
    public string Name { get; set; }

    public Person() { }
}

//...

var persons = OleDbFetch<Person>(
    r => new Person()
    { 
        ID = Convert.ToInt32(r["ID"]), 
        Name = r["Name"].ToString() 
    }, 
    connString, "SELECT ID, Name FROM Person");

Am I missing something obvious, being reckless, or giving myself too much trouble?

share|improve this question
    
I guess I should secure the connection... Make sure it's open. –  MPelletier Nov 10 '11 at 2:48

1 Answer 1

up vote 5 down vote accepted

Dispose() of types that implement IDisposable with using blocks:

using (var conn = new OleDbConnection(connectionString))
{
    conn.Open();
    using (var cmd = new OleDbCommand(query, conn))
    using (var reader = cmd.ExecuteReader())
    {
        while (reader.Read())
            yield return formator(reader);
    }
}
share|improve this answer
    
I'm a sucker for explicitly closing connections... –  MPelletier Nov 10 '11 at 3:50

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.