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

ADO.NET Wrapper

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

Problem

I end up writing a lot of the same code when I query a database I thought I would try to encapsulate that all in a class that could be used by any provider that implemented IDbConnection. i'm not looking to do much, return a DataTable when I want a result set, things like that. Here is my first attempt. I've tested that it works with SQL Server, but would like to know if there is any way to improve it or any issues I may not be aware of.

```
public class DataQuery where TConnection : IDbConnection, new()
{
private string connectionString;
private TConnection cnn;

private TConnection NewConnection()
{
cnn = new TConnection();
cnn.ConnectionString = connectionString;
return cnn;
}
public DataQuery(string connectionString)
{
this.connectionString = connectionString;
}

public int Execute(IDbCommand cmd)
{
using (cnn = NewConnection())
using (cmd)
{
cmd.Connection = cnn;
cnn.Open();

return cmd.ExecuteNonQuery();
}
}

public DataTable QueryDataTable(IDbCommand cmd)
{
using (cnn = NewConnection())
using (cmd)
{
cmd.Connection = cnn;
cnn.Open();

var t = new DataTable();
t.Load(cmd.ExecuteReader());
return t;
}
}

public T QueryValue(IDbCommand cmd)
{
using (cnn = NewConnection())
using (cmd)
{
cmd.Connection = cnn;
cnn.Open();

return (T)cmd.ExecuteScalar();
}
}

public IEnumerable QueryDataRecord(IDbCommand cmd)
{
using (cnn = NewConnection())
using (cmd)
{
cmd.Connection = cnn;
cnn.Open();

using (var reader = cmd.ExecuteReader())
{
while (reader.Read())
{
yield return reader;
}
}

Solution

using (cnn = NewConnection()) {}


This is a very dangerous design. Especially the private TConnection cnn field that is shared by each method.

If you ever use it in paralell then those methods will overwrite each other connections. You should use them locally only:

using (var cnn = NewConnection()) {}


using (cmd)


This is also a no go. I'd be really surprised when I found that my command has been disposed by the method using it. It is however a good idea to maintain the command by the method. It would be better if you hadn't to create it outside.

I suggest this design instead:

public int Execute(Action configureCommand)
{
    using (var cnn = NewConnection())
    using (var cmd = cnn.CreateCommand())
    {
        configureCommand(cmd);
        cmd.Connection = cnn;
        cnn.Open();

        return cmd.ExecuteNonQuery();
    }
}


then you can use it like this:

var execute = db.Execute(cmd => cmd.CommandText = "update Person.Address set PostalCode = PostalCode");


This way you don't have to think about which IDbCommand you need to create.

or with parameters:

var execute = db.Execute(cmd =>
{
    cmd.CommandText = "update Person.Address set PostalCode = PostalCode";
    var p1 = new SqlParameter("@p1", SqlDbType.VarChar);
    p1.Value = "abc";
    cmd.Parameters.Add(p1);
});

Code Snippets

using (cnn = NewConnection()) {}
using (var cnn = NewConnection()) {}
using (cmd)
public int Execute(Action<IDbCommand> configureCommand)
{
    using (var cnn = NewConnection())
    using (var cmd = cnn.CreateCommand())
    {
        configureCommand(cmd);
        cmd.Connection = cnn;
        cnn.Open();

        return cmd.ExecuteNonQuery();
    }
}
var execute = db.Execute(cmd => cmd.CommandText = "update Person.Address set PostalCode = PostalCode");

Context

StackExchange Code Review Q#141750, answer score: 4

Revisions (0)

No revisions yet.