HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Handle IDisposable objects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
idisposableobjectshandle

Problem

When I work a lot with objects, that implements IDisposable I am using this pattern, like shown here with a SqlConnection and a SqlCommand.

private string _connectionString = "";

    public IEnumerable GetUsers()
    {
        List users = new List();
        string sql = "SELECT * FROM [user]";
        InvokeSql(sql, (connection, command) =>
        {
            IDataReader reader = command.ExecuteReader();
            while (reader.Read())
            {
                User user = new User();
                users.Add(user);

                user.Name = reader["name"] as string;
            }
        });
        return users;
    }

    private void InvokeSql(string sql, Action action)
    {
        if (string.IsNullOrWhiteSpace(sql))
        {
            throw new ArgumentNullException("sql");
        }

        if (action == null)
        {
            throw new ArgumentNullException("action");
        }

        using (SqlConnection connection = new SqlConnection(_connectionString))
        using (SqlCommand command = new SqlCommand(sql, connection))
        {
            connection.Open();
            action(connection, command);
        }
    }


So I can reuse the InvokeSql and do not have to bother about the disposing.
But is it really a good idea, to use an Action in a using Statement?

Solution

So I can reuse the InvokeSql and do not have to bother about the disposing.

Sure. Except InvokeSql is private, so reuse is rather limited to that class. And if that class has a GetUsers method, I dare expect only User-related stuff in that class... which InvokeSql isn't.

In all likelihood, your model is going to involve more than just User entities; I'd make a base class to encapsulate the CRUD and this InvokeSql method; then a derived type can be responsible for User entities, and another derived type can be responsible for another entity type - something like a "repository".

ExecuteReader returns a SqlDataReader instance, which also implements IDisposable: you're missing a using block here:

IDataReader reader = command.ExecuteReader();
while (reader.Read())


using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
    ...

Code Snippets

IDataReader reader = command.ExecuteReader();
while (reader.Read())
using (SqlDataReader reader = command.ExecuteReader())
{
    while (reader.Read())
    ...

Context

StackExchange Code Review Q#126113, answer score: 5

Revisions (0)

No revisions yet.