Sunday, January 31, 2010

Day 19

Some serious refactoring and boolean reduction. My last post showed an attempt at making a long if statement more readable, using a technique that really didn't fit the situation. I used some hidden logic to generate some numbers with no clear meaning, to decide what action to take next. Though the action was clear, how I arrived on this action was not. I also drastically increased the amount of code in order to obtain only a bit more clarity.

Micah and I went back to thoroughly refactored and reduced the if statement. This was made quite easy because of a comprehensive test base, so that our refactoring could focus solely on simplification without worrying about function.

The process of simplication is pretty amazing. Though somewhat time consuming and attention demanding, it yielded some interesting results. As we refactored, we simply looked for patterns in the code, and through these patterns we searched for a higher level of abstraction. Bit by bit we were able to generalize and generalize until we would suddenly hit a new understanding, at which point great leaps of simplification could be made.

So after many itterations and cycles of refactoring we discovered what the logic really meant, and thus were able to greatly reduce the number of lines of code it took to express the same algorithm.

Once again, this is logic to decide what words need to be selected while dragging the mouse over some text after you double click and hold (in some word processors and this one, when you double click and drag the selection mode switches from individual characters to full words) . All of the 'isRightOf ' sorts of references refer to the position of the mouse in relation to the cursor and selected text.
Here was the process:

Starting with the If-Else chain I showed on my last post, we started refactoring out some of the conditionals into expressive methods:

public void invoke()
{
if (mouseIndex > boxInfo.findWordsRightEdge(boxInfo.cursorIndex))
{
if (mouseIndex > boxInfo.selectionIndex)
{
boxInfo.selectionIndex = boxInfo.cursorIndex;
boxInfo.cursorIndex = boxInfo.findWordsRightEdge(mouseIndex);
}
else if (boxInfo.cursorIndex < boxInfo.selectionIndex)
boxInfo.cursorIndex = boxInfo.findWordsLeftEdge(mouseIndex);
else
boxInfo.cursorIndex = boxInfo.findWordsRightEdge(mouseIndex);
}
else if (mouseIndex < boxInfo.findWordsLeftEdge(boxInfo.cursorIndex) && mouseIndex > boxInfo.selectionIndex)
{
boxInfo.cursorIndex = boxInfo.findWordsRightEdge(mouseIndex);
}
else if (mouseIndex < boxInfo.selectionIndex && mouseIndex < boxInfo.findWordsLeftEdge(boxInfo.cursorIndex))
{
if (boxInfo.cursorIndex > boxInfo.selectionIndex)
boxInfo.selectionIndex = boxInfo.cursorIndex;
boxInfo.cursorIndex = boxInfo.findWordsLeftEdge(mouseIndex);
}
}



With a limited understanding of what the true nature of this logic was, we tried several different naming schemes to see what worked:

if (isRightOfCursor())
{
if (isRightOfSelectionOnOriginalWord())
{
swapSelectionDirectionAndExtendToRight();
}
else if (isSelectionFacingLeft())
reduceSelectionFromLeft();
else
extendSelectionToRight();
}
else if (isLeftOfCursorInsideSelection())
{
reduceSelectionFromRight();
}
else if (isLeftOfSelection())
{
if (isSelectionFacingRight())
swapSelectionDirection();
extendSelectionToTheLeft();
}
}


Then we worked to get some symmetry to simplify it:

if (isRightOfCursor())
{
if (isInsideSelection)
reduceSelectionFromLeft();
else if(isRightOfSelection())
{
if (isSelectionFacingLeft())
swapSelectionDirection();
extendSelectionToRight();
}
}
else if (isLeftOfCursor())
{
if(isInsideSelection())
reduceSelectionFromRight();
else if (isLeftOfSelection())
{
if (isSelectionFacingRight())
swapSelectionDirection();
extendSelectionToTheLeft();
}
}



This new format gave us the idea that we have a head and a tail, since the Cursor is always at the head we could say the selection ends at the tail. Now we have a direction and position built into our abstraction:

if (isRightOfHead())
{
if (!isRightOfTail())
reduceSelectionFromLeft();
else if(isRightOfTail())
{
if (isSelectionFacingLeft())
swapSelectionDirection();
extendSelectionToRight();
}
}
else if (! isRightOfHead())
{
if(isRightOfTail())
reduceSelectionFromRight();
else if (!RightOfTail())
{
if (isSelectionFacingRight())
swapSelectionDirection();
extendSelectionToTheLeft();
}
}


A little Simplification:

if (isRightOfHead())
{
if (!isRightOfTail())
reduceSelectionFromLeft();
else
{
if (isSelectionFacingLeft())
swapSelectionDirection();
extendSelectionToRight();
}
}
else
{
if(isRightOfTail())
reduceSelectionFromRight();
else
{
if (isSelectionFacingRight())
swapSelectionDirection();
extendSelectionToTheLeft();
}
}



Now its clear that we are only doing 2 things. Moving the head and swapping the head and the tail (which is really just turning around). This allowed us to simplify the actions and lead us closer to a good solution. (also a little extra extracting methods to variables and such):

if (rightOfHead)
{
if (!rightOfTail)
repositionHead(boxInfo.findWordsLeftEdge(mouseIndex));
else
{
if (!selectionFacingRight)
turnAround();
repositionHead(boxInfo.findWordsRightEdge(mouseIndex));
}
}
else
{
if (rightOfTail)
repositionHead(boxInfo.findWordsRightEdge(mouseIndex));
else
{
if (selectionFacingRight)
turnAround();
repositionHead(boxInfo.findWordsLeftEdge(mouseIndex));
}
}


At last it is clear, for the most part, what is going on. We will always be moving the head, and we will be turning around only if the mouse is trailing the tail. If we are going to the right, we will always be positioning the cursor on the right edge of the word. Going to the left, the cursor is on the left edge of the word. This understanding leads us to these two boolean equations, and their respective actions:

boolean isMouseTrailingTheTail = selectionFacingRight && !rightOfTail || !selectionFacingRight && rightOfTail;
boolean isHeadingToTheRight = selectionFacingRight && !isMouseTrailingTheTail || !selectionFacingRight && isMouseTrailingTheTail;

if(isMouseTrailingTheTail)
turnAround();

if(isHeadingToTheRight)
repositionHead(boxInfo.findWordsRightEdge(mouseIndex));
else
repositionHead(boxInfo.findWordsLeftEdge(mouseIndex));


Then with some boolean arithmetic and De' Morgan's Law I found a remarkably simple and, after the fact, obvious solution:

boolean isMouseTrailingTheTail = selectionFacingRight && !rightOfTail || !selectionFacingRight && rightOfTail;
if(isMouseTrailingTheTail)
turnAround();

if(rightOfTail)
repositionHead(boxInfo.findWordsRightEdge(mouseIndex));
else
repositionHead(boxInfo.findWordsLeftEdge(mouseIndex));


A massive simplification, clarification, and reduction for what was once an incomprehensible If-Else chain. This new form is leaps and bounds better than the switch statement.

2 comments:

  1. This is even better than I imagined in my previous comment to your last post. Seems to me that you followed the trace of the structure evolving during your refactoring. Congratulations! What did you learn from this? :)

    ReplyDelete
  2. Quite a few things. One very important thing that I learned from this refactoring is that the code can often speak for itself. Rather than trying to fit the code and logic into a form you think makes sense, you can listen to the code and discover through deep inspection what actually makes sense. I tried to fit this square logic into a circular form. It just didn't fit. After some inspection I found that the code made a form of its own, and I only had to let it.

    The only issue was that it took a fair amount of time to find the right form. Though such refactoring is hugely beneficial, it just doesn't seem practical to spend so much time on every chunk of code you write. Maybe it will just get easier the more experienced I get, and the only way to get that experience is to do all the work; however, I still have to be getting production code done. I suppose this is probably an age old dilemma though

    ReplyDelete