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

When we left off yesterday, I mentioned that the rest of the day’s work on this refactoring (I do have a day job as well, please keep in mind!) I’d spend replacing the Grid.Columns[column][row]:Cell. Well in order to do that, I’ve introduced a new method in the IGrid interface:

public Cell GetCell(GridCoordinate coordinate)
{
    return Columns[coordinate.Column]
        [coordinate.Row];
}

This allows us to do some interesting things in the long term. But really it just makes parameter lists that are passed into methods cleaner.

Now we can get to the fun part – all those GetXKey methods… one for each direction. There’s so much duplication there! See if you can find the similarities in purpose and form between two of the four methods.

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 = location.MoveRight().CreateCellKey();
    }
    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 = location.MoveLeft().CreateCellKey();
    }
    return leftKey;
}

Any ideas? The problem is that there’s a lot of different code there. They’re looking at different criteria in that first boolean expression, but that logic is clear – is the cell (location) that’s being passed in outside the bounds of the Grid. If the cells a legitimate location, and meets the other criteria, then create the cell key.

So what we can introduce is a GridCoordinateBoundary class, representing the measurable borders (boundaries) of our Grid object.

public class GridCoordinateBoundary
{
    public int MinimumRow { get; set; }
    public int MaximumRow { get; set; }
    public int MinimumColumn { get; set; }
    public int MaximumColumn { get; set; }

    private GridCoordinateBoundary(int minimumRow, int maximumRow, int minimumColumn, int maximumColumn)
    {
        MinimumRow = minimumRow;
        MaximumRow = maximumRow;
        MinimumColumn = minimumColumn;
        MaximumColumn = maximumColumn;
    }

    public static GridCoordinateBoundary CreateFrom(int minimumRow, int maximumRow, int minimumColumn, int maximumColumn)
    {
        return new GridCoordinateBoundary(minimumRow, maximumRow, minimumColumn, maximumColumn);
    }
    
    public bool IsWithinBoundary(GridCoordinate location)
    {
        return (location.Row >= MinimumRow)
               && (location.Row < MaximumRow)
               && (location.Column >= MinimumColumn)
               && (location.Column < MaximumColumn);
    }
}

Now we can start pulling the logic together. We don’t need the 4 unique GetDownKey, GetUpKey, etc.

private static string GetKeyFor1(IGrid grid, GridCoordinate nextLocation, CellValues clickedValue)
{
    string key = String.Empty;
    if (grid.GetBoundary().
            IsWithinBoundary(nextLocation)
        && grid.GetCell(nextLocation).CellValue 
            == clickedValue
        && IsCellInMarkableState(
            grid.GetCell(nextLocation)))
    {
        key = nextLocation.CreateCellKey();
    }
    return key;
}

Note that the logic is right in the if block. Based on literature I’m currently reading (I’d highly recommend Robert C. Martin’s Clean Code so far), pulling the boolean expressions out for variable names is cleaner. This gives us the following result at the end of a short day’s work. I will spend the rest of the day getting the GetCell() method propagated to replace the Column[][] syntax that’s pervasive in the solution.

private static string GetKeyFor(IGrid grid, GridCoordinate nextLocation, CellValues clickedValue)
{
    string key = String.Empty;
    bool locationIsWithinGridBoundary = grid.GetBoundary().IsWithinBoundary(nextLocation);
    bool cellValueIsSameAsClicked = grid.GetCell(nextLocation).CellValue == clickedValue;

    if (locationIsWithinGridBoundary
        && cellValueIsSameAsClicked
        && IsCellInMarkableState(grid.GetCell(nextLocation)))
    {
        key = nextLocation.CreateCellKey();
    }
    return key;
}