The code I have been refactoring has been causing me a bit of pain, as I hinted in my last post. I have refactored plenty of good and bad code before. This time however, I headed off in the wrong direction too quickly. Before long, I had myself tangled and had to revert to the original code and start again. Before I get into my approach, let’s review the tools.

Refactoring is not something that should be done by hand, as there are very good tools available. I usually make heavy use of the stock ones in Visual Studio Team System – Software Developer Edition. I am using VS 2008, but not much in the Refactoring seems to have changed since VS 2005. Unfortunately, these are not available in VB.NET, but Microsoft does recommend using Refactor! for VB.NET which is available for free. I am fortunate to have access to the full version of Refactor!Pro and ReSharper. For me, I have actually uninstalled both of them, and just use the VS built in refactoring’s. Whenever you see a smart tag, Shift+Alt+F10, is your friend.

Refactor!Pro I find has a very clean interface and interaction. Their principle of no modal dialogs works very well to avoid jumping to the mouse. ReSharper has dynamic compilation as you type and it appears to have much smarter refactoring’s. On the negatives Refactor!Pro has a bit of a delay to give you the context menu of available refactoring’s, and nothing appearing really advanced like ReSharper. ReSharper though is bugging, crashes often (itself and VS), uses huge amounts of memory and slows down VS greatly. I feel it attempts to do a little too much, hijacking the intellisense and cripples VS when uninstalled. Additionally, due to the dependency that you get having these tools, I find myself feeling crippled when I have to work on another machine without them. I do have Refactor! for VB.NET installed, since there is no VS options and it is free, so I can install it on any machine I am working on. Although, if I have a choice, I would not opt for programming in VB. John Papa has an old but still relevant comparison if you want to read more. 

Armed with Refractor! for VB.NET my approach was as I normally would do and usually works well. Find obvious blocks of code that can easily be pulled out into using Extract Method. Doing this helps aid in understanding the code. Well-named methods become self commenting code. In this function there was a few For Each loops, some repeated. Removing the content of a loop into methods so that the start and finish can be seen on the one screen can often show possible optimizations that are not easily seen otherwise. While extracting a method in the initial state of this code I had seven parameters, half of them begin ref’s and the other being out’s. This makes for an extremely error prone function and the code more of a mess than it is already in. Extracting methods initially failed in this code due to the complete misuse of local variables.

The next Refactor command to come to the rescue is this case was, Move Declaration Near Reference. At the top of this function all the variables used (and not used) were declared. Not only that, but after a variable was used, instead of creating a new variable, it was just reassigned a value, not dependent on its previous value. This creates an unnecessary dependency, requiring an extract method attempt to return the value, when it is not really used after the method. As in the example below, extracting the first loop requires iRow and iSum to be returned.

Dim iRow As Integer
Dim iSum As Integer

For Each iRow = 0 In Table.Rows
    iSum += 1
Next
iSum = 0
For Each iRow = 0 In Table.Rows
    iSum += 2
Next

Reducing the scope of the variable to the smallest required, segments these two tasks. Before performing an extracted method this code should be refactored to:

Dim iSum As Integer
For Each iRow As Integer = 0 In Table.Rows
    iSum += 1
Next
Dim iSum As Integer = 0
For Each iRow As Integer = 0 In Table.Rows
    iSum += 2
Next

This will raise a warning that been Local variable ‘iSum’ is already declared in the current block. This will be fine once the code blocks have been extracted to methods. Declaring the variable again allows the refactoring tools to know that the variable is not required after the method. Meticulously performing this a great number of times to reduce the scope of the variables, and determine when a new declaration was required enable one particular method extraction to go from having a return value, 4 ref and 3 out parameters, to being a subroutine with no return value and no parameters.

In the example above, you could also see the possibility of combining the two loops. This could also be performed on the code I was working on, but was not immediately obvious until the contents of the loops were safely extracted. When refactoring, be careful not to automatically assume the previous programmer did not know what they were doing. There must be a reason why certain things were done. Do not just delete a block of code because you at first appearance seems unnecessary. It is likely you do not understand fully what it is doing or attempting to do. Reviewing code after it is written can reveal a great deal of improvements that can easily not be seen while you are in the process of writing it. It may just be an instance where the code built up over many iterations building on Technical Debt and was just never reviewed as a whole, allowing for great improvements with a little refactoring.