SQL connection in static class and wrapper

Question

I need to make some calls to stored procedures in my SQL database from a .NET application, using C#.

I have something like this, and I put just one of the methods but the rest will be similar:

public static class ClientDataAccess
{
 public static void AddClient(Client client)
 {
 var connectionString = ConfigurationManager.ConnectionStrings["appDatabase"].ConnectionString;
 using (var connection = new SqlConnection(connectionString))
 using (SqlCommand cmd = new SqlCommand("Client_add", connection) { CommandType = System.Data.CommandType.StoredProcedure })
 // Add parameters
 connection.Open();
 command.ExecuteNonQuery();
 }
 }
}
}

When I needed to test the classes where it's used I had to create a wrapper to be able to mock it:

public class ClientDataAccessWrapper : IClientDataAccessWrapper
{
 public void AddClient(Client client)
 {
 ClientDataAccess.AddClient(client);
 }
}

Is this considered to be a good practice?

• Jesse C. Slicer: Well, you need to wrap the var command = new SqlCommand bit in a using construct as well since it implements IDisposable just like SqlConnection.
• mitomed: True, didn't know that bit (I update it). So the rest it's more or less a good solution in your opinion

Answer 1

The class being static is not the only thing that makes testing more difficult then necessary.

var connectionString = ConfigurationManager.ConnectionStrings["appDatabase"].ConnectionString;

The hardcoded connection string name should be a parameter. You can have default constructor using a default connection string name but it's always a good idea to have another constructor that lets you use another value.

AddClient

If it's a ClientDatabase for CRUD about client objects then I find the client in name AddClient method redundant. Simply Add, Delete etc would be more appropriate.

By following this pattern you can have a single crud-generic interface for all types:

public interface IDataAccess<T>
{
 void Add(T obj);
 void Delete(T obj);
 ..
}

and later

public class ClientDataAccess : IDataAccess<Client>
{
 public void Add(Client obj) {..}
 public void Delete(Client obj) {..}
}

Answer 2

Thing is when I needed to test the classes where it's used I had to create a wrapper to be able to mock it.

Also, you have to create an instance of the wrapper in the production code, haven't you? So, what is the purpose of the static class then?

Static classes are inconvenient for unit testing. Instead of introducing wrappers that mimics the original class, you can simply get rid of statics at all and go with simple 'injectable' abstraction and implementation:

public class ClientDataAccess : IClientDataAccess
{
 public void AddClient(Client client)
 {
 // real implementation ...
 }
}