The Refactoring: Step 1

In our last code showing, we explored the code in question, marking it as a very suitable candidate for refactoring.

First things first, we have to be able to know what on earth is going on in this method – and that means descriptive variable names. When this refactoring session is complete, I’ll show the original code this was created from. Since my first source code (Java) was lost to the ages, I decompiled the class file – this resulted in the worst-of-the-worst as far as refactoring goes. Labels, and variable names like “l0” abound (that’s L-Zero for those of you using sans-serif fonts).

The most key renaming below is:

  • Changing parameter names. No more “gridX” and “gridY”, but rather “column” and “row”
  • Creating variables to represent boolean logic.
  • What does “row < grid.Width - 1” actually mean? (the row is not the last column == rowIsNotLastColumn – does this make sense? More on this later!)
  • Extraction of a method. IsCellInAMarkableState() encapulates the logic of checking to see if a given cell is in the Down or Normal states.

And here’s the first run at the refactoring:

private static bool IsCellInMarkableState(Cell c)
{
    return c.CellState == CellStates.Normal || c.CellState == CellStates.Down;
}


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

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

    int count = 1;

    string rightKey = String.Empty;
    string leftKey = String.Empty;
    string downKey = String.Empty;
    string upKey = String.Empty;

    bool rowIsNotFirstRow = row > 0;
    bool rowIsNotLastColumn = row < grid.Width - 1;
    bool rowIsUpToLastColumn = row <= grid.Width - 1;

    bool columnIsNotFirstColumn = column > 0;
    bool columnIsNotLastRow = column < grid.Height - 1;
    bool columnIsUpToLastRow = column <= grid.Height - 1;
    

    if (
        rowIsNotLastColumn 
        && columnIsNotFirstColumn
        && columnIsUpToLastRow
        && grid.Columns[row + 1][column].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[row + 1][column])
        )
    {
        downKey = CreateCellKey(row + 1, column);
    }
    if (
        rowIsNotFirstRow
        && columnIsUpToLastRow 
        && grid.Columns[row - 1][column].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[row - 1][column])
        )
    {
        upKey = CreateCellKey(row - 1, column);
    }
    if (
        rowIsNotLastColumn
        && columnIsNotLastRow 
        && grid.Columns[row][column + 1].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[row][column + 1])
        )
    {
        rightKey = CreateCellKey(row, column + 1);
    }
    if (
        rowIsUpToLastColumn 
        && columnIsNotFirstColumn
        && grid.Columns[row][column - 1].CellValue == clickedValue
        && IsCellInMarkableState(grid.Columns[row][column - 1])
        )
    {
        leftKey = CreateCellKey(row, column - 1);
    }


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

    return count;
}

Posted on 5/4/2009 7:12:13 PM by Jason Nadal

Permalink | Comments |

Categories:

Tags:

Be the first to rate this post

  • Currently 0/5 Stars.
  • 1
  • 2
  • 3
  • 4
  • 5
blog comments powered by Disqus