Restless C#ding

After Jetbrains’ recent releases of DotCover and Resharper 6, along with a mostly isolated set of libraries, I forced myself to completely cover the code. I started out with a fairly TDD set of libraries, which allow me to have some classes prepopulated based on contextual information from the System.Web.UI.Page object that will serve as the destination for communication between two pages.

To start out, I had several classes with their own unique responsibilities, and then one base page (which had a generic type constraint) that allowed for the automatic population of the object in the page.

public ReceivingPageBase<T> : System.Web.UI.Page
where T : class, IPagePopulatableObject, new()
{
public T RecievedObject {get;set;}
protected Override void OnPreLoad(EventArgs e)
{
//attempt to load the object, persist in viewstate;
//if postback, load it from viewstate instead.
}
}
public ReportingInformation : IPagePopulatableObject
{ //some properties, implementation of IPagePopulatableObject }

Now the tricky part. As I went to write tests, and kept analyzing code coverage, I was also reading through The Art of Unit Testing by Roy Osherove. The book’s very well paced, and a fairly light read. It’s pretty easily explained stubbing and mocking, which have eluded me for a bit of time. I had used constructor injection for my repositories and stubbed them out so that my tests were non-db-dependant, and got my overall coverage up to about 85%.

As an aside, I had run into a closing-brace bug with DotCover itself that was tricky to workaround (see my next post for some details on that particular gem).

As I went to write tests against the remaining bit of code (the base page shown in the code sample above), I had tons of trouble writing a stub that would essentially be a fake page. Oh how easy it would be if only I could tag System.Web.UI with an interface… but alas, it’s in a framework library and out of my control.

So I slept for a couple days on that particular issue, doing some research, when I came across one of Martin Fowler’s solutions to this particular issue… using a design pattern known as Passive View in order to wrap the page. The real system can use a concrete class that directly maps and wraps System.Web.UI.Page, while the tests can use a stub that simulates page context information. Let’s jump to code to see how it works (note the last image in the Passive View link was the clearest application to this problem). First, we need to isolate the ReceivingPageBase<T> itself from the loading of the object. The ReceivingPageBase<T> class will be moved out of the library and close to the applications which consume it. In it’s place will be:

//In library:
public PagePopulatableObjectReciever<T>
where T:class, IPagePopulatableObject, new()
{
private readonly Page _page;
public T RecievedObject {get;set;}
public PagePopulatableObjectReciever(Page page)
{
_page = page;
Received = DoLoadObject();
}
private T DoLoadObject()
{
//attempt to load the object from _page, 
//persist in _page.viewstate;
//if postback, load it from _page.viewstate instead.
}
}
//In application:
public AppSpecificRecievingPageBase<T> : System.Web.UI.Page
where T : class, IPagePopulatableObject, new()
{
public T RecievedObject {get;set;}
protected Override void OnPreLoad(EventArgs e)
{
//attempt to load the object, persist in viewstate;
//if postback, load it from viewstate instead.
}
}

Note that this only gets us halfway there. We’ve split apart the page from the receiving, so now the page is relatively ‘dumb’ (read: decoupled; SRP is good!). But this still couples the ObjectReciever to the System.Web.UI.Page, where we want to use some stub instead. Well, it’s time to apply the PassiveView pattern. Create the Passive View of System.Web.UI.Page, with an interface and a concrete.

//1. Create the passive view interface
public interface IStatefulPage
{
StateBag ViewState {get;}
bool IsPostBack {get;}
NameValueCollection QueryString {get;}
}
//2. Make the application base page conform
public AppSpecificRecievingPageBase<T> : 
System.Web.UI.Page, IStatefulPage
where T : class, IPagePopulatableObject, new()
{
public T RecievedObject {get;set;}
protected Override void OnPreLoad(EventArgs e)
{
//attempt to load the object, persist in viewstate;
//if postback, load it from viewstate instead.
}
//explicit interface declaration
//needed to avoid conflict with 
//existing ViewState variable
StateBag IStatefulPage.ViewState
{
get {return ViewState;}
}
public NameValueCollection QueryString
{
get { return Request.QueryString;}
}
//IsPostback is 'free'; 
//signature already conforms.
}
//3. Replace the ObjectReciever's page with IStatefulPage.
public PagePopulatableObjectReciever<T>
where T:class, IPagePopulatableObject, new()
{
private readonly IStatefulPage _page;
public T RecievedObject {get;set;}
public PagePopulatableObjectReciever(IStatefulPage page)
{
_page = page;
Recieved DoLoadObject();
}
private T DoLoadObject()
{
//attempt to load the object from _page, 
//persist in _page.viewstate;
//if postback, load it from _page.viewstate instead.
}
}

Now at this point, we’ve broken the coupling of object receiving from the Page object. This is fantastic… the last prep step is to create our stub to use for tests. Obviously depending on your tests, your behavior may vary, but I wanted to add in a sample set of data to receive, and have a working state to store it in for postback, giving me the following stub:

public class StubObjectReceivingPageWithValidObject :
IStatefulPage
{
public StubObjectReceivingPageWithValidObject
(string objectKey)
{
_queryString.Add("MASTERKEY",objectKey);
IsPostBack = false;
}
private readonly StateBag
_viewState = new StateBag();
public StateBag ViewState
{
get { return _viewState;}
}
public bool IsPostBack {get;set;}
private readonly NameValueCollection
_queryString = new 
NameValueCollection();
public NameValueCollection QueryString
{
get { return _queryString;}
}
}

Now I can use that stub for testing, and it will simulate page and context without the need for any sort of test web server, or ‘real’ building of a web page and context information.

What have we learned? Well, wrapping any framework class in an interface can be done through the use of wrapper objects, essentially making a Passive View. Our stub page winds up being a Test Double (aka Imposter) which allows us to test without using any true Page instances. That’s all good.

What’s the downside to doing this? Shallow interfaces for one. I made one that had ViewState, IsPostBack, and QueryString, because I knew those were what I needed to know. If some other class has other parts of the page, I could have either expanded IStatefulPage, or created another Passive View interface. Perhaps these can get out of control if you have too many. Also another downside is that I’m using trainwreck notation (see number 5 here from “Uncle” Bob Martin’s seminal Clean Code) for things like mapping IStatefulPage’s QueryString to Request.QueryString. I can live with that, at least for the present.

All of this leads to increasing testability, and speed of testing. It’s important to keep test speeds high in order to keep your team running them often.

This post is a continuation of our previous article, a response to this smelly code.

Remember those columns and rows everywhere, and how tough it was to swap them everywhere? Well this is a prime time to fix that. How else can we reduce parameter count, while still passing in the same information? Enter complex object.

In this case, we can gain:

• Reduction of column / row parameters to just location
• Give the CreateCellKey(x,y) method a home (really, it gives a key for a particular coordinate.

So our first step is to create the new object and move in the CreateCellKey method; note this is also implementing a static create method:

public class GridCoordinate
{
    public int Column { get; set;}
    public int Row { get; set; }

    public static string CreateCellKey(GridCoordinate location)
    {
        return location.Column + "," + location.Row;
    }

    public static GridCoordinate Create(int column, int row)
    {
        return new GridCoordinate {Column = column, Row = row};
    }
}

And then we have to fix the calls that just deal with columns and rows. Note the code is longer for now, but we’ll address this with reusability in the near future!

Our resulting changed code is as follows:

private static string GetRightKey(IGrid grid, GridCoordinate location, CellValues clickedValue)
{

    string rightKey = String.Empty;
    bool columnIsNotLastColumn = location.Column < grid.Width - 1; ;
    if (columnIsNotLastColumn
        && grid.Columns[location.Column + 1][location.Row].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[location.Column + 1][location.Row]))
    {
        rightKey = GridCoordinate.CreateCellKey(GridCoordinate.Create(location.Column + 1, location.Row));
    }
    return rightKey;
}

private static string GetLeftKey(IGrid grid, GridCoordinate location, CellValues clickedValue)
{
    string leftKey = String.Empty;
    bool columnIsNotFirstColumn = location.Column > 0;
    if (columnIsNotFirstColumn
        && grid.Columns[location.Column - 1][location.Row].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[location.Column - 1][location.Row]))
    {
        leftKey = GridCoordinate.CreateCellKey(GridCoordinate.Create(location.Column - 1, location.Row));
    }
    return leftKey;
}

private static string GetDownKey(IGrid grid, GridCoordinate location, CellValues clickedValue)
{
    string downKey = String.Empty;
    bool rowIsNotLastRow = location.Row < grid.Height - 1;
    if (rowIsNotLastRow
        && grid.Columns[location.Column][location.Row + 1].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[location.Column][location.Row + 1]))
    {
        downKey = GridCoordinate.CreateCellKey(GridCoordinate.Create(location.Column, location.Row + 1));
    }
    return downKey;
}

private static string GetUpKey(IGrid grid, GridCoordinate location, CellValues clickedValue)
{
    string upKey = String.Empty;
    bool rowIsNotFirstRow = location.Row > 0;
    if (rowIsNotFirstRow
        && grid.Columns[location.Column][location.Row- 1].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[location.Column][location.Row - 1]))
    {
        upKey = GridCoordinate.CreateCellKey(GridCoordinate.Create(location.Column, location.Row - 1));
    }
    return upKey;
}

private int IfContainsClickedValueThenMark(string key, IGrid grid, GridCoordinate location, CellValues clickedValue)
{
    int countOfMarkedCells = 0;
    if (key != String.Empty && !_searchedCells.Contains(key))
    {
        _searchedCells.Add(key, String.Empty);
        countOfMarkedCells += MarkConnectingCellsRecursive(grid, location, clickedValue);
    }
    return countOfMarkedCells;
}

private int MarkConnectingCellsRecursive(IGrid grid, GridCoordinate location, CellValues clickedValue)
{
    //set the current location to be clicked.
    grid.Columns[location.Column][location.Row].CellState = CellStates.Down;

    //we seed the count at one here, because at least the current cell is to be
    //marked.
    int countOfMarkedCells = 1; 

    //look all around our current cell for other cells of the same value.
    string rightKey = GetRightKey(grid, location, clickedValue);
    string leftKey = GetLeftKey(grid, location, clickedValue);
    string downKey = GetDownKey(grid, location, clickedValue);
    string upKey = GetUpKey(grid, location, clickedValue);

    //if the keys ARE the same value, look around them for the same value, mark them, and add them
    //to the count
    countOfMarkedCells += IfContainsClickedValueThenMark(rightKey, grid, GridCoordinate.Create(location.Column + 1, location.Row), clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(leftKey, grid, GridCoordinate.Create(location.Column - 1, location.Row), clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(downKey, grid, GridCoordinate.Create(location.Column, location.Row + 1), clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(upKey, grid, GridCoordinate.Create(location.Column, location.Row- 1), clickedValue);

    return countOfMarkedCells;
}

Now that we’ve done that, we can really do some cool things. Since we have a reference to a distinct location as an object, that object knows where it is. You can treat it like a plotter. Given a location, move up, down, left, right.(Note, we’ll probably want to rename this object to something like GridPlotter, so it makes more sense… this would have a GridCoordinate object, but for now, we’ll place the methods right in the GridCoordinate object:

public GridCoordinate MoveUp()
{
    return Create(Column, Row - 1);
}

public GridCoordinate MoveDown()
{
    return Create(Column, Row + 1);
}

public GridCoordinate MoveLeft()
{
    return Create(Column - 1, Row);
}

public GridCoordinate MoveRight()
{
    return Create(Column + 1, Row);
}

public string CreateCellKey()
{
    return CreateCellKey(this);
}

This lets you refactor code like follows. Instead of the +1 and –1 everywhere:

countOfMarkedCells += 
    IfContainsClickedValueThenMark(
    rightKey, grid, 
    GridCoordinate.Create(location.Column + 1, 
        location.Row), 
    clickedValue);

You can just put:

countOfMarkedCells += IfContainsClickedValueThenMark(
    rightKey, grid, 
    location.MoveRight(), 
    clickedValue);

At this point, unit tests all still pass, and app feels pretty good still, so this is a pretty successful refactoring process thus far. For the time until the next post, I’ll be making sure all areas of code referencing this class (the GridController class) no longer use pure “int column, int row” values, but rather use the more robust GridCoordinate class.

After this is complete, the GridCoordinate class will be renamed, and co-located near the Grid itself. Right now, this is only referenced by the GridController, but in reality, the GridCoordinate should serve as the base of all Grid cell lookups. (Rather than always using the Grid.Columns[column][row]:Cell).

I've had pretty much my entire adult life as experience in diagnosing, troubleshooting, and usually repairing various computer woes. The best lessons were usually after catastrophic mistakes (which I committed, naturally).

 When I was about 13, I managed to reverse polarity on a 4-pronged power cord, and shorted out my pc. The magic smoke escaped (it's blue, apparently!), and a loud snap & that burning smell you never want to associate with hardware were suddenly present. Needless to say there were casualties. Surprisingly, most everything survived -- except for the power supply (to be fair, it was mostly dead to start with) and the hard drive. This was when I was heavy into Pascal programming, and I lost quite a bit of computer time. When you're only 13, replacing a power supply and a hard drive take months (the drive was only 40mb, but weighed about 15 pounds). I learned my lesson about polarities, though!

So how does this relate to development? Well, if you write code that performs poorly, there's obvious repercussions. In a similar vein, when code that is not scalable, or scales poorly reaches production, the flaws can become readily apparent. The ideal scenario would be to be able to recognize poor code / poor situations by sight, and have a common plan of action to address them.

Enter "Anti-patterns". These are an attempt to document types of common mistakes seen in the wild, so to speak, and give a sample way to refactor to 'better' code. Someone has actually registered "antipatterns.com", to address some of those concerns. These may be with development, and they also may be with project managment. Not everything is on their site, but what is there is pretty insightful. Check it out here... 

I was talking with a colleague today about code patterns. How can one find a pattern based on code? Especially for things that seem to be very reused? There's the standard Gang of Four book for the obvious cases (MVC, Observer, Memento and the like), but what about those obscure cases?

We ran into a hacky solution for grouping enum values together in order to sort them by functionality. So if I have enum values of Bar1, Bar2, and Bar3, all Bar enum values and IFoo objects of types Foo1, Foo2, and Foo3, where Foo1 has a property of (Bar1 | Bar2), Foo2 has (Bar2 | Bar3) and Foo3 has (Bar1 | Bar3), how best to represent those?

Or the better question -- how to deduce a pattern from existing code? For "nonstandard" patterns?