views:

56

answers:

2

If I had the code below what are the best practise / design considersations for replacing it with IoC (we are looking to use Castle Windsor). As the "using" statement is responsible for creating the connection object, you can't inject that directly into the constructor or the method. Note: Using SQL connection as what appears to be a clean example, the main benefit here is mocking/unit testing

public void CustomerRespository
{
    ....
    public void Save(Customer customer)
    {
       using (var cn = new SqlConnection(connectionString))
       {
           using (var cm = new SqlCommand(commandString, cn))
           {
              ....
              cn.Open();
              cm.ExecuteNonQuery();
           }
       }
    }
}

I believe there would be at least a few options, but as we're just starting out with IoC I'm not confident that they wouldn't cause us problems later on and/or fly in the face of IoC concepts. My favoured approach would be too modify the approach as follows, can anyone highlight potential issues with it?

public interface IDatabase
{
    IDbConnection Connection(string connectionString);
    IDbCommand Command(string text, IDbConnection conn);
}

public class SqlDB : IDatabase
{
    IDbConnection Connection(string connectionString)
    { return new SqlConnection(connectionString); }

    IDbCommand Command(string text, IDbConnection conn)
    { return new SqlCommand(text, conn); }
}

public interface ICustomerRespository
{
   void Save(Customer customer)
}

public class CustomerRespository : ICustomerRespository
{
    public IDatabase DB{get; private set;}

    public CustomerRespository( IDatabase db)
    {
       DB = db;
    }

    ....
    public void Save(Customer customer)
    {
       using (var cn = DB.Connection(connectionString))
       {
           using (var cm = DB.Command(commandString, cn))
           {
              ....
              cn.Open();
              cm.ExecuteNonQuery();
           }
       }
    }
}
+1  A: 

I have used IoC but not Castle although they are all similar so here is my catch on this.

I think you are on the right track - although I might use a separate factory for connection and command or in fact leave the connection opening and command running to another class so repository does not have to know this detail. Just have IDatabase in the constructor of your class so that it is injected (or use property if you are using property-based injection). Replace SqlConnection and SqlCommand in your code with IDbConnection and IDbCommand.

UPDATE

They do inherit/implement IDisposable so you CAN use using statement. Sorry for my mistake.

Aliostad
@Alistad, I've just taken a look at MSDN definition for the interfaces, and that says the two mentioned do inherit from IDisposable. http://msdn.microsoft.com/en-us/library/system.data.idbconnection.aspx and http://msdn.microsoft.com/en-us/library/system.data.idbcommand.aspx
Paul Hadfield
Sorry yes, my object explorer did not show me this, funny...
Aliostad
+1  A: 

The general approach seems fine to me, although I wouldn't try to mock the IDbConnection and IDbCommand interfaces since that could be complicated, and more importantly won't tell you whether the code works as intended.

It would allow you to change the database you use however, so you could use something like Sqlite for unit testing, and then test the code against your production database during integration testing.

You could also move the connection string into the IDatabase abstraction which would simplify the clients.

Lee
@Lee: Good idea to sort out the connection string, I'd kind of left that bit as is to keep example code reasonable length. Using Moq I wouldn't be looking to mock the entire IDbConnection or repository, just the bits that were relevant to the test (i.e. time out or unavailable).
Paul Hadfield