So after the last step, we start to see some real problems. Let’s take a look at a chunk of code – the first real logic block:

if (
    rowIsNotLastColumn 
    && columnIsNotFirstColumn
    && columnIsUpToLastRow
    && grid.Columns[row + 1][column].CellValue == clickedValue
    && IsCellInMarkableState(grid.Columns[row + 1][column])
    )
{
    downKey = CreateCellKey(row + 1, column);
}

What you see here is that something’s seriously wrong. Unless we are doing something crazy, why on earth would we be checking for something like “row is not last column”. Can you think of a scenario that this ever makes sense?

In the context of this method, the answer is a resounding “no”. So what’s actually going on here? Well, one thing we can do is to fix things to what you think they should be.

This block of code attempts to check to see if you have a cell that:

1. Can have a cell below it
2. Has a cell below it with the same cell value as the value that the user originally clicked.

So, the resulting code that you expect to be correct is:

if  (
rowIsNotLastRow
&& columnIsNotFirstColumn //why? more smell?
&& columnIsUpToLastRow //munster cheese? more smell...
&& grid.Columns[row + 1][column].CellValue == clickedValue
&& IsCellInMarkableState(grid.Columns[row + 1][column])
)
...

Now when I try to run this, unit tests start failing, and running the program results in random cells being marked all over the grid. So the extent of this problem reaches quite far.

I’ll skip to the chase here… because of gridX and gridY’s pervasiveness, coupled with the seemingly random placement of ordering those items as parameters within method signatures, these values are getting swapped all over the place… it’s only due to the seemingly lucky smelly code above that this happens to work.

The fix here is to go up and down and make sure that the parameters are in the same order throughout the chain (more on this in a later article in this series!), as well as making sure everything uses the more descriptive “row” and “column” variable names.

So, we wind up with the code below. Note the lack of use of descriptive variables to replace the inline boolean logic, but at least the code makes sense!

private int MarkConnectingCellsRecursive1(IGrid grid, int column, int row, CellValues clickedValue)
{

    //set the current location to be clicked.
    grid.Columns[column][row].CellState = CellStates.Down;

    int count = 1;

    string rightKey = String.Empty;
    string leftKey = String.Empty;
    string downKey = String.Empty;
    string upKey = String.Empty;
    
    if (column < grid.Width - 1 && row >= 0 && row <= grid.Height - 1 && grid.Columns[column + 1][row].CellValue == clickedValue
        && (grid.Columns[column + 1][row].CellState == CellStates.Normal || grid.Columns[column + 1][row].CellState == CellStates.Down))
    {
        rightKey = CreateCellKey(column + 1, row);
    }
    if (column > 0 && row >= 0 && row <= grid.Height - 1 && grid.Columns[column - 1][row].CellValue == clickedValue
        && (grid.Columns[column - 1][row].CellState == CellStates.Normal || grid.Columns[column - 1][row].CellState == CellStates.Down))
    {
        leftKey = CreateCellKey(column - 1, row);
    }
    if (column >= 0 && column <= grid.Width - 1 && row < grid.Height - 1 && grid.Columns[column][row + 1].CellValue == clickedValue
        && (grid.Columns[column][row + 1].CellState == CellStates.Normal || grid.Columns[column][row + 1].CellState == CellStates.Down))
    {
        downKey = CreateCellKey(column, row + 1);
    }
    if (column >= 0 && column <= grid.Width - 1 && row > 0 && grid.Columns[column][row - 1].CellValue == clickedValue
        && (grid.Columns[column][row - 1].CellState == CellStates.Normal || grid.Columns[column][row - 1].CellState == CellStates.Down))
    {
        upKey = CreateCellKey(column, row - 1);
    }


    if (rightKey != String.Empty && !_searchedCells.Contains(rightKey))
    {
        //if the one to the right is a match and we haven't looked around it yet,                                
        _searchedCells.Add(rightKey, String.Empty);
        count += MarkConnectingCellsRecursive(grid, column + 1, row, clickedValue);
    }
    if (leftKey != String.Empty && !_searchedCells.Contains(leftKey))
    {
        //if the one to the left is a match                
        _searchedCells.Add(leftKey, String.Empty);
        count += MarkConnectingCellsRecursive(grid, column - 1, row, clickedValue);
    }
    if (downKey != String.Empty && !_searchedCells.Contains(downKey))
    {
        //if the one below is a match                
        _searchedCells.Add(downKey, String.Empty);
        count += MarkConnectingCellsRecursive(grid, column, row + 1, clickedValue);
    }
    if (upKey != String.Empty && !_searchedCells.Contains(upKey))
    {
        //if the one above is a match                
        _searchedCells.Add(upKey, String.Empty);
        count += MarkConnectingCellsRecursive(grid, column, row - 1, clickedValue);
    }
    return count;
}