Refactoring has become a common practice amongst many developers I work with.  Why refactor?

Well, first it takes the pressure of off design phase of project in a sense.  You do not have spend ton of time upfront designing the code and assuring that you thought of every detail before you even write a single line of code.  You simply get a rough design and start coding right away, knowing that as you implement the system, get more familiar with the code, you will notice better patterns and refactor the code towards them.

We all know that, if done right, refactoring leads to simpler smaller code base that is easier to maintain.  Meantime we use Unit Tests to assure that the refactoring we performed did not change the outcome or results that our code produces.  I have been following this practice for a while and it has kept me out of trouble.

However what I realized over time is that the Unit Tests themselves become more complex, with too much duplication, and too hard to read.

Take for example this problem I had recently at the client I am working for.  We inherited a “failed” project (story of my life – I always end up coming in to fix someone else’s mess, but enough about that), a project that  another consulting company has given up on after complicating the matter a bit first.

No, they did not use code refactoring, they did not have unit tests, instead they have used code generation tool to “save time”.  It is just that their code generation templates were not really thought through.  Now we have a complex system, with a lot of bugs to clean and lot of code to refactor.  And yes, in order to refactor we had to start writing unit tests to support it.

Well all of their Business Objects are initialized by making a DB call in their constructor that uses Datasets as the DTOs that transfer that data from the DAL (I know what you are going to say now – but sometimes you have to work with what you have – we have to bring this site online!).

But lets start with an example.  Lets say I had a bussiness object called ShoppingCart and that it contained a list Product sand a list of Options for eaach product.  In the constructor of the ShoppingCart, they would have an instance of a ShoppingCartData object that would have a method callled ExecuteDataSet() which although the name does not state it returns all Products and their options that belong to this shopping cart.  Then the code inside of the dataset woule lop through the tables and populate both object lists, something like:

ShoppingCartData _shoppingCartData = new ShoppingCartData();

public ShoppingCart()

{

     DataSet ds = _shoppingCartData.ExecuteDataSet();

     foreach(DataRowView in ds.Tables[0].Rows){

…

}

So in order to write tests against these objects I had to mock the DAL call and replace the DataSet that the DB would return with my own.   As you can see ShoppingCartData was not injected, nor did it have an interface so for mocking part I had to bring the big guns – TypeMock. 

Mock data = MockManager.Mock(typeof(ShoppingCartData));

cartData.ExpectAndReturn("ExecuteDataSet",cartDataSetStub); 

So mocking the Db call and taking DB out of eqation was easy.  Even generating the stub data for the tests was not the problem.  You might have read one of my previos blogs where I wrote about this little code generation tool that helped me generate DataTable Stub objects just by runnig the sql :

http://www.nermins.net/post/2007/07/Mock-ADONET-with-ease-using-IDataReader-Stub-objects.aspx

In that previous example I take advantage of DataTable CreateReader() method to generate IDataReader Stubs.  However in this case my Stub objects are DataSets, so I can use these table Stubs directly.

I also have the code for that tool available on the google code site:

http://code.google.com/p/data-stub-generator/

So, if setting up mocking and setting up Stub data was not the problem then what was? I had to write a number of tests against each of the BO including Shopping Cart.  That meant setting up the data for the cartTadaSetsTub DataSet.  I also wanted my code genaration tool to generate tables with one and couple of tests records that represent the dafault /valid data, and then explicitly set the values/cells that were needed for the test in the test itself.

For example let’s say that we have the rule that says that shopping cart can not check out if there is at least on item that has been discontinued since we placed it on the shopping cart.  That means that my table returning Products data would have to have one record that has “Discountinued” column set to true.  So let’s take a look at the code needeed for that:

DataSet cartDataSetStub = new DataSet();

DataTable products = new ShoppingCartProductsStub();

products.AddDefaultRow();

products.Rows[0][“Discountinued”] = true;

DataTable options = new ShoppingCartOptionsStub();

cartDataSetStub.Tables.Add(products);

cartDataSetStub.Tables.Add(options);

 

Mock cartData = MockManager.Mock(typeof(ShoppingCartData));

//Assure that _shoppingCartData.ExecuteDataSet()

//returns our cartDataSetStub instead of calling DB

cartData.ExpectAndReturn("ExecuteDataSet",cartDataSetStub); 

ShoppingCart cart = new ShoppingCart(); 

Assert.That(cart.CanCheckOut, Is.EqualTo(false));  

First 7 lines of code are there just to simply setup “fake” output from the database.  There is more code in the part that sets up the data for the test than the actual test.  And actually it could have been worse if I have not used the generated table stub objects ShoppingCartProductsStub and ShoppingCartOptionsStub.  All that code crowds the test and doesn’t really expresss my intention – it is hard to read.  So what did I do to solve that?

Fluent Interfaces to the rescue!  How about this for a change:

Mock cartData = MockManager.Mock(typeof(ShoppingCartData));

cartData.ExpectAndReturn(

    "ExecuteDataSet",

     Stub.GetDataSet(

         ShoppingCartProductsStub.Empty().AddDefaultRow()

            .AtRow(0)

            .InCell(“Discountinued”)

            .SetValue(true),

         ShoppingCartOptionsStub.Empty())); 

 

ShoppingCart cart = new ShoppingCart(); 

Assert.That(cart.CanCheckOut, Is.EqualTo(false)); 

Four statements above are functionaly equivalent to that code mesh in previous example.  And as you can see you can simply read the code to figure what it does!  We are generating DataSet with two tables where on the first table we add the default row of data and then set the cell “Discontinued” to false.  Second table is empty.  And that is it.

So how do these fluent interfaces work?  What is the logic behind them?  Well simply put, lest take a look at the methods that we use to manipulate an object  (StubTable in this case).  In the example above those methods are:  Empty(), AddDefaultRow(), AtRow(int rowNo), InCell(string cellName), SetValue(object value).  Generaly these methods would return void.  In fluent programing they return the object itself or better an interface that implements these other methods.  So first I created the object called StubTable:

public class StubTable : DataTable

{

    private int _currRow = 0;

    private string _currCell = string.Empty;

    protected StubTable(){}

    public static StubTable Empty()

    {

        return new StubTable();

    }

    public StubTable AtRow(int rowNo)

    {

        _currRow = rowNo;

        return this;

    }

    public StubTable InCell(string cellName)

    {

        _currCell = cellName;

        return this;

    }

    public StubTable SetValue(object value)

    {

        Rows[_currRow][_currCell] = value;

        return this;

    }

    public StubTable AddRow(params object[] values)

    {

        Rows.Add(values);

        return this;

    } 

}

  

Then the generated SoppingCartProducts and ShoppingCartOptions DataTables inherit from Stub table and are modified to look like this:

public class ShoppingCartProductsStub : StubTable

{

    public new static ShoppingCartProductsStub Empty()

    {

        return new ShoppingCartProductsStub();

    }

    protected ShoppingCartProductsStub()

    {

        InitColumns();

    }

    private void InitColumns()

    {

        Columns.Add("ShopingCartID", typeof(Int32));

        Columns.Add("ProductID", typeof(Int32));

        Columns.Add("ProductName", typeof(String));

        Columns.Add("ProductNumber", typeof(String));

        Columns.Add("ProductQty", typeof(Int32));

        Columns.Add("Price", typeof(Decimal));

        Columns.Add("PromotionPrice", typeof(Decimal));

        Columns.Add("Discontinued", typeof(Boolean));

        …

    }

    public StubTable AddDefaultProduct()

    {

        Rows.Add(705582, 1, "Round Cook-N-Dine Built-in Cook Top", "MO-60", 5,

          decimal.Parse("1200.9400000000000"), decimal.Parse("1200.9400000000000"),false);

        return this;

    }

}

And Finaly Our Stb class that builds and returns the DataSet DTO:

public class Stub

{

    public static DataSet GetDataSet(params DataTable[] tables)

    {

        DataSet ds = new DataSet();

        foreach (DataTable table in tables)

            ds.Tables.Add(table);

        return ds;

    }

} 

The conclusion I would draw from this is that with a little thinking upfront, and a little refactoring we can make our tests a lot more readable.  We always have to keep in mind that our tests might be the first thing that the next developer is going to look at.  Making the test little bit more readible helps them figure out easier on how the actual object being tested is used and what are the expectations set for it.

 

Before I start, I would like to point out that if you are confused about differences between Mock and Stub objects, please read the Fowler’s post on the subject:

http://www.martinfowler.com/articles/mocksArentStubs.html

I have seen too many “Unit tests” where developers do not isolate the test to the unit (object/method) being tested.  Yes they are testing a business object, but to test its behavior they load half of the object hierarchy in the project.  Moreover if your object gets initialized from external resource like a database, then they create this complex “test databases” that contain their test data.  These databases have to be shared with other developers.  If you have a continuous build environment that runs tests as a part of the build process, then you have to have a copy of the test database there.  All these test databases have to be modified as you modify schema/data in your development database.  In addition if your test modifies data in the database then you need to setup an additional process in the SetUp or Teardown to restore the data to initial state.

So the test should be simpler if we mock the db dependency, right?  So what does that involve?  Let’s say we have an object called project Project with constructor as described below:

public Project(IProjectGateway gateway, int id)

{

    using(IDataReader dr = gateway.GetProjectBy(id)) {

        if(dr.Read()) {

            _id = dr.GetInt32(0);

            _name = dr.GetString(1);

            _date = dr.GetDateTime(2);                   

        }

    }

}

 

Where IProjectGateway is an interface defining a set of methods/and object in charge of persistence of the Project data to and from the database.    So lets look how that interface might look like:

public interface IProjectGateway {

    IDataReader GetProjectBy(int id);

    ...

}

So far this is simple, right? Actual implementation of that interface does not matter for our test.  Why?  Because we are not testing database, its resources/file storage, hardware, connection pooling, network connections, etc.  All we are supposed to test is that our object properly populates its field from a returned DataReader.  

To test the Project object without the database roundtrip we will only need to mock the IProjectGateway, setting the expectations for the GetProjectBy(id) to return our test data (data reader).  I have to mention that the dynamic mocks in the examples bellow were done using my favorite mocking tool TypeMock.Net.

[Test]

public void AssureFetchMapsFields()

{

    Mock<IProjectGateway> projGatewayMock = MockManager.Mock<IProjectGateway>();

 

    projGatewayMock.ExpectAndReturn("GetProjectBy", new ProjectDataStub().CreateDataReader());

   

    Project project = new Project(projGatewayMock.MockedInstance,1);

 

    Assert.AreEqual(1,project.Id);

    Assert.AreEqual("Test",project.Name);

    Assert.AreEqual(DateTime.Parse("1/1/2000"),project.Date);

}

 

So lets see what exactly have we done in the test above.  First line creates our dynamic mock instance.  Second line is the interesting part.  It states that we expect one method call on our IProjectGatewayMock, and that is “GetProjectBy()” method.  Once it is called we want the IDataReader to be returned from ProjectDataStub.CreateDataReader().  The rest of the code instantiates the Project object and then assures that the Project’s properties are initialized.

But what about this ProjectDataStub.CreateDataReader()?  Well I have noticed the ability of the DataTable objects to create an instance of TableDataReader(which implements IDataReader).  So theoretically we could create a DataTable with column types that reflect the types of the actual columns of the table/view we are fetching from db and populate this DataTable with test record(s).  Then our mock IProjectGateway can return the IDataReader from this object, and voila – no db or any other external connection used in the test.

So if we follow this logic the ProjectDataStub should be an object that inherits from DataTable and populates its columns and rows with test records when it is constructed.  But coding these stub object for each and every test might be a bit tedious.  To solve this problem I have created a rather simple tool that allows us to generate this DataStub simply by copying and pasting select SQL statements from the fetch Stored Procedure and executing it.

Above is the scren shot showing how tool works and its output.  If you think that this tool might be useful for you feel free to download the code from the link bellow:

StubGenerator.zip (780.62 KB)

Is this all when it comes to testing the DAL?  No, obviously we have only tested the data mapping part.  Code in this Project constructor can throw SqlExcpetion (database down, network problem, schema problem).  We need to assure that our code handles that.

Now putting the try/ctach block in the Project constructor does not make sense - I do not want Project instantiated if we were unable to retreive its data.  So how do we test this case?  Lets assume that our application is using Model View Presenter architecture, and that the object instantiating our Project is the Presenter object of the View that displays Project.  Obviously we need to assure that this Presenter can recover from the SqlException thrown by Project.  Lets take a look at the code bellow:

public class ProjectPresenter

{

....

public void DisplayProject(){

    try{

        Project p = new Project();

 

        ... do something with project like display it on the view etc...

 

    }catch(SqlException e){

        Log.LoqException(e);       

    }

}

So if we can assure that the Log.LogException(e) is called when the Project throws the SqlException, that would be a proof that the exception was handled.  Keep in mind that in Test Driven Development we would be writting the test prior to the DisplayProject() method being written (and the catch block inside of it existing)

Dynamic mocking helps us here also.  In this case we are testing the Presenter, which means that the other two objects Log and Project would be mocked.  Test would encompass mocking ProjectConstructor instead of returning Project throws SqlException, and then assuring that the Log Mock accepts the call to LogException(e).

In addition there would be tests that we need to run for Insert, Update and Delete (if Project needs to support that), but I will leave that for a future post.

I have received few comments on the first post, one of them being from Rocky Lhotka the creator of the Csla.Net framework.  He basically pointed to his advanced data sample (DeepData.sln available for download at www.lhotka.net). 

The idea is that if we encapsulate all of the ADO.NET constructs required to fetch/update a single table into a “Data object” and move it from the Fetch() method of Csla object (some might find this similar to Table Data Gateway pattern), then the only thing we have to mock is that Data object. 

In addition setting expectations would be lot simpler, since everything is encapsulated.  So, instead of me talking about it lets look at how that changes the Fetch method of the ProjectList class defined in PTracker sample:

private void Fetch(string nameFilter) {

     RaiseListChangedEvents = false;

     DataFactory df = new DataFactory();

     using(ProjectListData data = df.GetProjectListDataObject()) {

         SafeDataReader dr = data.GetProjectList();

         IsReadOnly = false;

         while (dr.Read()) {

             ProjectInfo info = new ProjectInfo(

               dr.GetGuid(0),

               dr.GetString(1));

               // apply filter if necessary

               if ((nameFilter.Length == 0) || (info.Name.IndexOf(nameFilter) == 0))

                 Add(info);

         }

         IsReadOnly = true;

     }

     RaiseListChangedEvents = true;

}

Code above is simpler than the original or the refactored code I had (only a single using block, and no ADO.NET dependencies) .  So lets take a look at what happened.  We stopped using the Database class (that functionality will move into our Data object – ProjectListData).  We can see 2 new objects constructed in this code: 1.       DataFactory – Factory in charge of instantiating all of the Data objects for our project 2.       ProjectListData – Data object, whose purpose is to encapsulate the ADO.NET constructs, and return a SafeDataReader back to the Ftech() method. 

It is important to note that ProjectListData implements IDisposable interface.  That way as we dispose of it, it will dispose corresponding DataReader, DbCommand and a Connection.  Hence only one using block needed here. Test are then as simple as:

[Test]

public void LoadsOne() {

     Mock mockProjectListData = MockManager.Mock(typeof (ProjectListData));

     mockProjectListData.ExpectAndReturn("GetProjectList",

         new ProjectListFetchOneDRStub().GetDataReaderStub());

     mockProjectListData.ExpectCall("Dispose");

     ProjectList item = ProjectList.GetProjectList();

     Assert.AreEqual(1,item.Count);

}  

[Test(Description = "DataReader returns 3 items but only one should be inserted, based on filter")]

public void LoadsThreeFiltersTwo() {

     Mock mockProjectListData = MockManager.Mock(typeof(ProjectListData));

     mockProjectListData.ExpectAndReturn("GetProjectList",

         new ProjectListFetchThreeDRStub().GetDataReaderStub());

     mockProjectListData.ExpectCall("Dispose");

     ProjectList item = ProjectList.GetProjectList("test");

     Assert.AreEqual(1, item.Count);

}  

As you can see mocking of the fetch process and replacing of the SafeDataReader is only couple of lines of code.  First line defines that we intend to mock ProjectList object.  Second one sets expectation that the method called GetProjectList() will be called.  Result of that call is supposed to be replaced by result of the call to  ProjectListFetchOneDRStub().GetDataReaderStub() in the first test, or the ProjectListFetchThreeDRStub().GetDataReaderStub() in the second test. 

These two methods return our stub DataReaders that contain the test data (source code for these is available in my previous post).  And that is it.  Rather simple!

At the end let me show you the code of our ProjectListData object:

public class ProjectListData : IDisposable {

     private SqlConnection _cn;

     private SqlCommand _cm;

     private SafeDataReader _data;

     private bool disposed;

     internal ProjectListData()

     {

         _cn = new SqlConnection(Database.PTrackerConnection);

         _cm = _cn.CreateCommand();

         _cm.CommandType = CommandType.StoredProcedure;

         _cm.CommandText = "getProjects";

         _data = new SafeDataReader(_cm.ExecuteReader());

     }

     public SafeDataReader GetProjectList()

     {

         return _data;

     }

     #region IDisposable Members

     public void Dispose()

     {

         Dispose(true);

         GC.SuppressFinalize(this);

     }

     protected virtual void Dispose(bool disposing)

     {

         if (!disposed) {

             if (disposing) {

                 // Dispose managed resources.

                 _data.Dispose();

                 _cm.Dispose();

                 _cn.Dispose();

             }

             // Dispose unmanaged resources

         }

         disposed = true;

     }

     #endregion

}

 

I must admit that I am a huge fan of Test Driven development.  Although I generally use CRC process to design my classes and their interactions, I tend to use TDD process to fine-tune the collaborations between these objects, and generally simplify my initial object design.  While I know that there are many skeptics in usefulness of TDD as a software design process, I believe that they underestimate one important factor in developer’s arsenal: laziness!  After a while each one of us tries to find a way to minimize the number of tests they have to write per object you implement.  If you build it in its simplest form, minimizing interactions and state permutations you minimize the number of the tests.  In addition tests help us understand how objects is used/how it behaves/interacts with other objects.  And most importantly test that we write give us that safety net to facilitate change, refactor code without constant worry that we might break existing functionality.  

Mocks on the other hand are the mechanism that assures that tests focus on the unit/component being tested and not the components that this unit interacts with.  When the tested code uses other components things can get quite fragile and the tests will quickly become system tests.  Examples are classes that use a database, a logging system, web service and any other external components.  These external components might require a complex setup. For example, in order to run a test that uses a database, we must have a running database with the tables and data setup correctly for the test (we must cleanup this data before each test runs).  Too much work for a lazy developer, and more importantly test results easily compromised by factors that are out of our control.

So mocking in this case sounds like a valid solution.  But what is the problem in implementing this in a CSLA world?  Well, CSLA has this concept of ‘Mobile Objects’, which means that the object is generally created in the (App) Server context, and then serialized to the client.  For example let’s take a look at the way we retrieve ProjectList object from the sample ProjectTracker20cs project.

/// <summary>Return a list of all projects.</summary>

public static ProjectList GetProjectList()

{

return DataPortal.Fetch<ProjectList>(new Criteria());

}

or,

/// <summary>Return a list of projects filtered by project name.</summary>

public static ProjectList GetProjectList(string name)

{

    return DataPortal.Fetch<ProjectList>

      (new FilteredCriteria(name));

}

For all of us that use CSLA daily this means that the DataPortal is going to instantiate the object of type ProjectList, call appropriate override of the method DataPortal_Fetch() defined in the ProjectList, and finally serialize it back to the client where that instance is returned trough the factory method in question. 

DataPortal_Fetch method overrides should resemble something like this:

private void DataPortal_Fetch(Criteria criteria)

{

    // fetch with no filter

    Fetch("");

}

private void DataPortal_Fetch(FilteredCriteria criteria)

{

    Fetch(criteria.Name);

}

private void Fetch(string nameFilter)

{

    RaiseListChangedEvents = false;

    using (SqlConnection cn = new SqlConnection(Database.PTrackerConnection)) {

        cn.Open();

        using (SqlCommand cm = cn.CreateCommand()) {

            cm.CommandType = CommandType.StoredProcedure;

            cm.CommandText = "getProjects";

            using (SafeDataReader dr = new SafeDataReader(cm.ExecuteReader())) {

                IsReadOnly = false;

                while (dr.Read()) {

                    ProjectInfo info = new ProjectInfo(

                      dr.GetGuid(0),

                      dr.GetString(1));

                    // apply filter if necessary

                    if ((nameFilter.Length == 0) || (info.Name.IndexOf(nameFilter) == 0))

                        Add(info);

                }

                IsReadOnly = true;

            }

        }

    }

    RaiseListChangedEvents = true;

}

Now let’s take a look at how most of the mocking frameworks mock dependencies.  Generally one needs to create an interface on the object we are mocking, and then pass the reference to that interface to the object we are trying to test.  This is essentially the implementation of the “Dependency injection pattern”, where the actual implementation is passed as this interface at run time, or mock object during the tests.  This, as far as I can tell, applies to NMock, EasyMock, or RhinoMock.

So the problem is that these mocking tools require us to build “mockable” objects.   This will not work with CSLA, or at least will not be implemented easily.  The problem is that, if one wants to mock dependencies such as data access layer components (db connection, data reader, command) one would have to instantiate them on the client side and then “inject” them as arguments into the Factory Method being called, making sure that they are somehow serialized to the server side.  And naturally database components are not serializable.

So, is it possible to mock CSLA dependencies that reside only on server side and are not serialized back and forth?  The answer is “Yes”.  Yes if you use TypeMock.Net library.  What is the major advantage of this tool when compared to other .Net mocking tools?  TypeMock uses Aspect Oriented technology to redirect calls from the real code to the mock object instantiated.  So how does this work?

Before we try to mock dependencies we have to remember one of the golden rules of Mocking:  “Never to Mock classes/interfaces you do not own or have source code to”.  What does that mean?   I have seen code where developers tried to Mock interfaces as IDataReader (or SqlDataReader) for example.  That interface is too complex and it is not something I defined or have control over.  So the first thing I would like to do is build a class called Database, that encapsulates all of the database communication in the Fetch() method.  So let’s do a little refactoring:

private void Fetch(string nameFilter)

{

    RaiseListChangedEvents = false;

    using (Database db = new Database(Database.PTrackerConnection)) {

        SqlCommand cm = db.CreateSPCommand("getProjects");

        using (SafeDataReader dr = db.ExecuteSafeDataReader(cm)) {

            IsReadOnly = false;

            while (dr.Read()) {

                ProjectInfo info = new ProjectInfo(

                  dr.GetGuid(0),

                  dr.GetString(1));

                // apply filter if necessary

                if ((nameFilter.Length == 0) || (info.Name.IndexOf(nameFilter) == 0))

                    Add(info);

            }

            IsReadOnly = true;

        }

    }

    RaiseListChangedEvents = true;

}

Let us compare the code above to the original Fetch().  One can notice that instead of creating SqlConnection object we created a Database object passing its constructor a desired connection string.  You will also notice that we are not explicitly “Opening” database connection.  That is because the Database class is managing SqlConnection internally and opening it as needed (for example, within the Database.ExecuteSafeDataReader() call as you might notice bellow when we take a look at the Database class).   

In addition you will notice that we have removed the using block around the SqlCommand instance.  Again the Database instance manages SqlCommand objects it creates and disposes them at the time it is disposed itself (together with disposing a SqlConnection).

 Below is a simplified version of the Database class.  Some of you might notice s similarity to the Database class implemented in Enterprise Library.