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

When we last left off, the method was much smaller; we had extracted methods for the entire first half of the method. This article focuses on the bottom half. Note that the if blocks in the bottom half check to see that:

1. The key is not empty.
2. The cell corresponding to the key has not already been checked
3. If 1&2 are fine, then check around THAT cell for matching cells (from the originally clicked cell)
4. If there are any new connecting cells, add them to the scoring tally.

So, we can pull out a method and pass in the next one to check. Here’s our new method:

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

Note that this is the first time in this series that we’ve actually reduced the number of lines of code. Most of the time, this will wind up happening, however in some cases (like this method), it takes some work to get the code to where you can actually see the similarities in logic. Duplicate code does not always look like duplicate code at first glance! Here’s the resulting code in the method we’re focusing on for the refactoring. We’ve also made the following changes:

1. Changed “count” to something more meaningful: it’s actually the count of connected (marked) cells.
2. We’ve added some descriptive comments. Note that this can mean that our code has room for further optimization. If our code needs the comments, how well is it written?

private int MarkConnectingCellsRecursive(IGrid grid, int column, int row, CellValues clickedValue)
{
    //set the current location to be clicked.
    grid.Columns[column][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, column, row, clickedValue);
    string leftKey = GetLeftKey(grid, column, row, clickedValue);
    string downKey = GetDownKey(grid, column, row, clickedValue);
    string upKey = GetUpKey(grid, column, row, 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, column + 1, row, clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(leftKey, grid, column - 1, row, clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(downKey, grid, column, row + 1, clickedValue);
    countOfMarkedCells += IfContainsClickedValueThenMark(upKey, grid, column, row - 1, clickedValue);

    return countOfMarkedCells;
}