Extract method

Last post indicated a gotcha that can occur when reordering parameters in a signature. Here's one that can come up with Extract Method. Like the last example this one occurred during actual coding and it would have been prevented had I had these refactoring tools.

Say I had the following (contrived) code:

class BinaryNode {
     private BinaryNode left;
     private object value;
     private BinaryNode right;
     private int height;
     private Color color;

     public BinaryNode(BinaryNode right, object value, BinaryNode left) {
           //initialize fields
           int newHeight = 0;
           //logging stuff
           //validation stuff
           newHeight = max(right.height, left.height) + 1;
           //more stuff
           //figure out color
           height = newHeight;

Now, that constructor is kind of huge and I want to break it down into manageable chunks. In order to do i start cutting and pasting code and manually figuring out the signature and then passing the parameters correctly and dealing with how to get any return values. in the above example, i went about doing that and i ended up with a new method that looked like:

void Initialize(..., int newHeight) {
    newHeight = max(right.height, left.height) + 1;

I then did a few more manual extracts. My constructor now looked great. It had a very readable structure: "VerifyArguments; CalculateHeight; CalculateColor; etc." Each method also looked pretty reasonable. The problem was that nothing was working properly. Basic issue (which looks obvious in the code above, but which i completely missed), was that I passed the newHeight argument and ended up copying it, so the assignment which happened in the constructor now happened in another method and didn't affect the constructor's local parameter at all.

The different you get by using the IDE's built in support for this is that:

we will we make it significantly easier to extract a method.
we will do it correctly.

In the case above we would have generated a method like:

void Initialize(..., ref int newHeight) { ... }

By passing as ref we would have kept the semantics necessary to keep your code behaving the same way as it did before.

When you have a huge method that you really want to make manageable it really changes things when you can just "extract, extract, extract" and haev something much more simple. After that you can do some "reorder parameters" to make things make more sense and further make your code easier to understand.