Programming Tools: Refactoring

Refactoring is the process of modifying code without affecting
either its input or output; that is, the code's interface remains the
same. How do we know that refactoring works? The only objective way to
find the answer is by testing the code before and after refactoring
occurs. The results should match exactly. This makes testing an integral
part of any refactoring effort--you cannot successfully separate the two.

Some confusion exists between what it means to refactor code as opposed
to reworking it. Refactoring is a subset of reworking. If you are making no changes
to the interface of a program, you are refactoring. If you add or change
the interface, you are rewriting the code. Strictly speaking, when you
do both together, you are rewriting. However, for political reasons,
you still might want to call it refactoring.

When it comes to methodology, refactoring usually is done in small
steps. After each step, you run those unit tests originally written
for the existing code. Repeat these two steps as needed. Also, a number
of refactoring patterns can be followed. Martin Fowler's book (see
Resources) contains some basic ones, and Joshua Kerievsky's more recent
text (see Resources) contains some more advanced ones. As a side issue,
it is interesting to note that most books and products dealing with
refactoring use Java as the language of choice. Is that because Java is
the most appropriate language, or does Java code need more refactoring?

All the refactoring tools I have seen so far require heavy human
intervention. They don't do pattern recognition, so they aren't able
to see commonality in code or design.
Java Tools
As noted above, a lot of refactoring tools are out there, especially for
Java. Open-source tools are available, some of which are listed in
Resources. Commercial refactoring products can be found in the usual
way. In this article, I discuss two open-source tools, Eclipse
and Bicycle Repair Man. Eclipse is designed for use with Java, while
Bicycle Repair Man is designed to be used with Python.

Let's start with Eclipse and Java. Eclipse offers an impressive set
of refactoring facilities, including support for:

• Copying and moving Java elements
• Extracting a method
• Extracting a local variable
• Extracting a constant
• Renaming a package
• Renaming a compilation unit
• Renaming a class or interface
• Renaming a method
• Renaming a field
• Renaming a local variable
• Renaming method parameters
• Changing method signature
• Inlining a local variable
• Inlining a method
• Inlining a constant
• Self encapsulating a field
• Replacing a local variable with a
  query
• Pulling members up to superclass
• Pushing members down to subclasses
• Moving static members between types
• Moving an instance method to a
  component
• Converting a local variable to a field
• Converting an anonymous inner class to a nested
  class
• Converting a nested type to a top level
  type
• Extracting an interface from a type
• Replacing references to a type with references to one of
  its subtypes

To give you some idea of what Eclipse's refactoring can do, I chose at
random the Java program shown in Listing 1. It outputs an About dialog
box.
Listing 1. Random Java
Program

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");

        Button button= new Button("Close");
        button.addActionListener(
            new ActionListener() {
                public void actionPerformed(ActionEvent e) {
                    dispose();
                }
            }
        );

        Label label1= new Label("JUnit");
        label1.setFont(new Font("dialog", Font.PLAIN, 36));

        Label label2= new Label("JUnit "+
                                Version.id()+ 
                                " by Kent Beck and Erich Gamma");
        label2.setFont(new Font("dialog", Font.PLAIN, 14));

        Logo logo= new Logo();

        GridBagConstraints constraintsLabel1= new GridBagConstraints();
        constraintsLabel1.gridx = 3; constraintsLabel1.gridy = 0;
        constraintsLabel1.gridwidth = 1; constraintsLabel1.gridheight = 1;
        constraintsLabel1.anchor = GridBagConstraints.CENTER;
        add(label1, constraintsLabel1);

        GridBagConstraints constraintsLabel2= new GridBagConstraints();
        constraintsLabel2.gridx = 2; constraintsLabel2.gridy = 1;
        constraintsLabel2.gridwidth = 2; constraintsLabel2.gridheight = 1;
        constraintsLabel2.anchor = GridBagConstraints.CENTER;
        add(label2, constraintsLabel2);

        GridBagConstraints constraintsButton1= new GridBagConstraints();
        constraintsButton1.gridx = 2; constraintsButton1.gridy = 2;
        constraintsButton1.gridwidth = 2; constraintsButton1.gridheight = 1;
        constraintsButton1.anchor = GridBagConstraints.CENTER;
        constraintsButton1.insets= new Insets(8, 0, 8, 0);
        add(button, constraintsButton1);

        GridBagConstraints constraintsLogo1= new GridBagConstraints();
        constraintsLogo1.gridx = 2; constraintsLogo1.gridy = 0;
        constraintsLogo1.gridwidth = 1; constraintsLogo1.gridheight = 1;
        constraintsLogo1.anchor = GridBagConstraints.CENTER;
        add(logo, constraintsLogo1);

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }
}

First, I scanned the code and saw that two similar fragments
of code dealt with labels. Second, I manually brought together all
the code to do with labels. Using similar logic, I brought together the
code dealing with the button and the logo. This step is shown in Listing
2.
Listing 2. Rearranging the Code

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");

        Label label1= new Label("JUnit");
        label1.setFont(new Font("dialog", Font.PLAIN, 36));

        Label label2= new Label("JUnit "+
                    Version.id()+ 
                    " by Kent Beck and Erich Gamma");
        label2.setFont(new Font("dialog", Font.PLAIN, 14));

        Logo logo= new Logo();

        GridBagConstraints constraintsLabel1= new GridBagConstraints();
        constraintsLabel1.gridx = 3; 
        constraintsLabel1.gridy = 0;
        constraintsLabel1.gridwidth = 1; 
        constraintsLabel1.gridheight = 1;
        constraintsLabel1.anchor = GridBagConstraints.CENTER;
        add(label1, constraintsLabel1);

        GridBagConstraints constraintsLabel2= new GridBagConstraints();
        constraintsLabel2.gridx = 2; 
        constraintsLabel2.gridy = 1;
        constraintsLabel2.gridwidth = 2; 
        constraintsLabel2.gridheight = 1;
        constraintsLabel2.anchor = GridBagConstraints.CENTER;
        add(label2, constraintsLabel2);

        Button button= new Button("Close");
        button.addActionListener(
            new ActionListener() {
                public void actionPerformed(ActionEvent e) {
                    dispose();
                }
            }
        );
        GridBagConstraints constraintsButton1=new GridBagConstraints();
        constraintsButton1.gridx = 2; 
        constraintsButton1.gridy = 2;
        constraintsButton1.gridwidth = 2; 
        constraintsButton1.gridheight = 1;
        constraintsButton1.anchor = GridBagConstraints.CENTER;
        constraintsButton1.insets= new Insets(8, 0, 8, 0);
        add(button, constraintsButton1);

        GridBagConstraints constraintsLogo1= new GridBagConstraints();
        constraintsLogo1.gridx = 2; 
        constraintsLogo1.gridy = 0;
        constraintsLogo1.gridwidth = 1; 
        constraintsLogo1.gridheight = 1;
        constraintsLogo1.anchor = GridBagConstraints.CENTER;
        add(logo, constraintsLogo1);

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }
}

Next, I extracted each method by first highlighting each method and then
selecting Refactor | Extract Method. The dialog box is shown below.

The resulting code is shown in Listing 3.
Listing 3. Refactored Code

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");


        gbLabel1();
        gbLabel2();
        gbButton();
        gbLogo();

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }

        /**
         *
         */
        private void gbLogo() {
            Logo logo= new Logo();
            GridBagConstraints constraintsLogo1= new GridBagConstraints();
            constraintsLogo1.gridx = 2; 
            constraintsLogo1.gridy = 0;
            constraintsLogo1.gridwidth = 1; 
            constraintsLogo1.gridheight = 1;
            constraintsLogo1.anchor = GridBagConstraints.CENTER;
            add(logo, constraintsLogo1);
        }

        /**
         *
         */
        private void gbButton() {
            Button button= new Button("Close");
            button.addActionListener(
                new ActionListener() {
                    public void actionPerformed(ActionEvent e) {
                        dispose();
                    }
                }
            );
            GridBagConstraints constraintsButton1= new GridBagConstraints();
            constraintsButton1.gridx = 2; 
            constraintsButton1.gridy = 2;
            constraintsButton1.gridwidth = 2; 
            constraintsButton1.gridheight = 1;
            constraintsButton1.anchor = GridBagConstraints.CENTER;
            constraintsButton1.insets= new Insets(8, 0, 8, 0);
            add(button, constraintsButton1);
        }

        /**
         *
         */
        private void gbLabel2() {
            Label label2= new Label("JUnit "+
                                    Version.id()+ 
                                    " by Kent Beck and Erich Gamma");
            label2.setFont(new Font("dialog", Font.PLAIN, 14));
            GridBagConstraints constraintsLabel2= new GridBagConstraints();
            constraintsLabel2.gridx = 2; 
            constraintsLabel2.gridy = 1;
            constraintsLabel2.gridwidth = 2; 
            constraintsLabel2.gridheight = 1;
            constraintsLabel2.anchor = GridBagConstraints.CENTER;
            add(label2, constraintsLabel2);
        }

        /**
         *
         */
        private void gbLabel1() {
            Label label1= new Label("JUnit");
            label1.setFont(new Font("dialog", Font.PLAIN, 36));
            GridBagConstraints constraintsLabel1= new GridBagConstraints();
            constraintsLabel1.gridx = 3; 
            constraintsLabel1.gridy = 0;
            constraintsLabel1.gridwidth = 1; 
            constraintsLabel1.gridheight = 1;
            constraintsLabel1.anchor = GridBagConstraints.CENTER;
            add(label1, constraintsLabel1);
        }
}

Looking at the code again, it seems that gbLabel1 and gbLabel2
share the same logic, but the values of some of the variables differ
slightly. Manually comparing the two, I tried to create a new method based
on gbLabel1 that had parameters on those variable values that differed
between gbLabel1 and gbLabel2. I did this by using the Introduce Parameter
refactoring wizard. Listing 4 shows the result of using this wizard to
replace the label's text and font values with method parameters.
Listing 4. Replacing Values with Parameters

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");

        gbLabel1(new Label("JUnit"),new Font("dialog",Font.PLAIN, 36));
        gbLabel2();
        gbButton();
        gbLogo();

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }

        /**
         *
         */
        private void gbLogo() {
            Logo logo= new Logo();
            GridBagConstraints constraintsLogo1= new GridBagConstraints();
            constraintsLogo1.gridx = 2; 
            constraintsLogo1.gridy = 0;
            constraintsLogo1.gridwidth = 1; 
            constraintsLogo1.gridheight = 1;
            constraintsLogo1.anchor = GridBagConstraints.CENTER;
            add(logo, constraintsLogo1);
        }

        /**
         *
         */
        private void gbButton() {
            Button button= new Button("Close");
            button.addActionListener(
                new ActionListener() {
                    public void actionPerformed(ActionEvent e) {
                        dispose();
                    }
                }
            );
            GridBagConstraints constraintsButton1= new GridBagConstraints();
            constraintsButton1.gridx = 2; 
            constraintsButton1.gridy = 2;
            constraintsButton1.gridwidth = 2; 
            constraintsButton1.gridheight = 1;
            constraintsButton1.anchor = GridBagConstraints.CENTER;
            constraintsButton1.insets= new Insets(8, 0, 8, 0);
            add(button, constraintsButton1);
        }

        /**
         *
         */
        private void gbLabel2() {
            Label label2= new Label("JUnit "+
                                    Version.id()+ 
                                    " by Kent Beck and Erich Gamma");
            label2.setFont(new Font("dialog", Font.PLAIN, 14));
            GridBagConstraints constraintsLabel2= new GridBagConstraints();
            constraintsLabel2.gridx = 2; 
            constraintsLabel2.gridy = 1;
            constraintsLabel2.gridwidth = 2; 
            constraintsLabel2.gridheight = 1;
            constraintsLabel2.anchor = GridBagConstraints.CENTER;
            add(label2, constraintsLabel2);
        }

        /**
         * @param lblText TODO
         * @param lblFont TODO
         *
         */
        private void gbLabel1(Label lblText, Font lblFont) {
            Label label1= lblText;
            label1.setFont(lblFont);
            GridBagConstraints constraintsLabel1= new GridBagConstraints();
            constraintsLabel1.gridx = 3; 
            constraintsLabel1.gridy = 0;
            constraintsLabel1.gridwidth = 1; 
            constraintsLabel1.gridheight = 1;
            constraintsLabel1.anchor = GridBagConstraints.CENTER;
            add(label1, constraintsLabel1);
        }
}

Two things in Listing 4 are worth noting. First, in the comments before the newly modified method,
TODOs remind you to comment on the meaning of the parameters. Second, in
both gbLabel1 and gbLabel2 the placement of the label is specified using
a rectangle with its upper-left corner and height and width values. Eclipse cannot recognize these
as attributes of a rectangle; it must be done manually. I used the Change Method Signature
wizard to introduce the Rectangle parameter.

Besides changing the method signature, I also needed to change manually
the code where gbLabel1 is invoked. Listing 5 shows the results with
some minor formatting changes being added.
Listing 5. Changing gbLabel1 Invocation Code

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");


        gbLabel1(new Label("JUnit"),
             new Font("dialog", Font.PLAIN, 36),
             new Rectangle(3,0,1,1));
        gbLabel2();
        gbButton();
        gbLogo();

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }

        /**
         *
         */
        private void gbLogo() {
            Logo logo= new Logo();
            GridBagConstraints constraintsLogo1= new GridBagConstraints();
            constraintsLogo1.gridx = 2; 
            constraintsLogo1.gridy = 0;
            constraintsLogo1.gridwidth = 1; 
            constraintsLogo1.gridheight = 1;
            constraintsLogo1.anchor = GridBagConstraints.CENTER;
            add(logo, constraintsLogo1);
        }

        /**
         *
         */
        private void gbButton() {
            Button button= new Button("Close");
            button.addActionListener(
                new ActionListener() {
                    public void actionPerformed(ActionEvent e) {
                        dispose();
                    }
                }
            );
            GridBagConstraints constraintsButton1= new GridBagConstraints();
            constraintsButton1.gridx = 2; 
            constraintsButton1.gridy = 2;
            constraintsButton1.gridwidth = 2; 
            constraintsButton1.gridheight = 1;
            constraintsButton1.anchor = GridBagConstraints.CENTER;
            constraintsButton1.insets= new Insets(8, 0, 8, 0);
            add(button, constraintsButton1);
        }

        /**
         *
         */
        private void gbLabel2() {
            Label label2= new Label("JUnit "+
                                    Version.id()+ 
                                    " by Kent Beck and Erich Gamma");
            label2.setFont(new Font("dialog", Font.PLAIN, 14));
            GridBagConstraints constraintsLabel2= new GridBagConstraints();
            constraintsLabel2.gridx = 2; 
            constraintsLabel2.gridy = 1;
            constraintsLabel2.gridwidth = 2; 
            constraintsLabel2.gridheight = 1;
            constraintsLabel2.anchor = GridBagConstraints.CENTER;
            add(label2, constraintsLabel2);
        }

        /**
         * @param lblText Text to appear in label
         * @param lblFont Font to use in showing text
         * @param lblRect the size and position of the label
         *
         */
        private void gbLabel1(Label lblText, Font lblFont, Rectangle lblRect) {
            Label label1= lblText;
            label1.setFont(lblFont);
            GridBagConstraints constraintsLabel1= new GridBagConstraints();
            constraintsLabel1.gridx = lblRect.x;
            constraintsLabel1.gridy = lblRect.y;
            constraintsLabel1.gridwidth = lblRect.width;
            constraintsLabel1.gridheight = lblRect.height;
            constraintsLabel1.anchor = GridBagConstraints.CENTER;
            add(label1, constraintsLabel1);
        }
}

Next, I noticed that the variable label1 could be replaced by label,
because only one variable of this type is in this method. To do this,
I used the Rename... wizard. I highlighted the name to be changed and
invoked the wizard, as shown below.

Because the gbLabel1 method now has become quite generic, we also can
change its name simply to gbLabel. We start by highlighting an instance of
its name and invoking the Rename... wizard. Next, we replace the call to
gbLabel2 with an equivalent call to gbLabel. Again, this must be done manually.
Also, because gbLabel2 is no longer needed, we should delete it. Listing 6
shows the result of all this.
Listing 6. Cleaning Up the Code

package junit.awtui;

import java.awt.*;
import java.awt.event.*;

import junit.runner.Version;

class AboutDialog extends Dialog {
    public AboutDialog(Frame parent) {
        super(parent);

        setResizable(false);
        setLayout(new GridBagLayout());
        setSize(330, 138);
        setTitle("About");

        gbLabel(new Label("JUnit"),
            new Font("dialog", Font.PLAIN, 36),
            new Rectangle(3,0,1,1));
        gbLabel(new Label("JUnit "+
                          Version.id()+ 
                          " by Kent Beck and Erich Gamma"),
            new Font("dialog", Font.PLAIN, 14),
            new Rectangle(2,1,2,1));
        gbButton();
        gbLogo();

        addWindowListener(
            new WindowAdapter() {
                public void windowClosing(WindowEvent e) {
                    dispose();
                }
            }
        );
    }

        /**
         *
         */
        private void gbLogo() {
            Logo logo= new Logo();
            GridBagConstraints constraintsLogo1= new GridBagConstraints();
            constraintsLogo1.gridx = 2; 
            constraintsLogo1.gridy = 0;
            constraintsLogo1.gridwidth = 1; 
            constraintsLogo1.gridheight = 1;
            constraintsLogo1.anchor = GridBagConstraints.CENTER;
            add(logo, constraintsLogo1);
        }

        /**
         *
         */
        private void gbButton() {
            Button button= new Button("Close");
            button.addActionListener(
                new ActionListener() {
                    public void actionPerformed(ActionEvent e) {
                        dispose();
                    }
                }
            );
            GridBagConstraints constraintsButton1= new GridBagConstraints();
            constraintsButton1.gridx = 2; 
            constraintsButton1.gridy = 2;
            constraintsButton1.gridwidth = 2; 
            constraintsButton1.gridheight = 1;
            constraintsButton1.anchor = GridBagConstraints.CENTER;
            constraintsButton1.insets= new Insets(8, 0, 8, 0);
            add(button, constraintsButton1);
        }

        /**
         * @param lblText Text to appear in label
         * @param lblFont Font to use in showing text
         * @param lblRect the size and position of the label
         *
         */
        private void gbLabel(Label lblText, Font lblFont, Rectangle lblRect) {
            Label lbl= lblText;
            lbl.setFont(lblFont);
            GridBagConstraints constraintsLabel1= new GridBagConstraints();
            constraintsLabel1.gridx = lblRect.x;
            constraintsLabel1.gridy = lblRect.y;
            constraintsLabel1.gridwidth = lblRect.width;
            constraintsLabel1.gridheight = lblRect.height;
            constraintsLabel1.anchor = GridBagConstraints.CENTER;
            add(lbl, constraintsLabel1);
        }
}

If you wish, you can continue on the same track with both the button and
logo related methods. However, it is more of the same, so I won't bore
you further on using refactoring with Eclipse and Java.
Python Tools
The standard refactoring tool for Python is Bicycle Repair Man (BRM). Its
feature set is more limited and less mature than Eclipse's set, in part
because of the language differences and in part because of the number
of developers working on them. Bicycle Repair Man's current refactoring
wizards are Rename, Extract method, Inline variable, Extract variable,
Move class and Move function.

For this test, I chose a program I knew something about--the token
handler for PyMetrics. Last month I wrote about code metrics
and PyMetrics. I did not like how I coded the PyMetrics routine
shown below, and its McCabe metric of 52 for the __call__ method gave me reason to refactor and
rework the code. Recall that the McCabe Cyclomatic metric measures the
number of paths through your function or procedure. At 52, this method
is considered extremely high risk and untestable. Listing 7 shows the __call__ method that I wanted refactored.
Listing 7. PyMetric __call__ Method

""" TokenHandler readies tokens to passing to metric routines.

  $Id: tokenhandler.py,v 1.2 2005/01/19 06:13:33 rcharney Exp $
"""
__version__ = "$Revision: 1.2 $"[11:-2]

#import token
from globals import *

class TokenHandler( object ):
  """ Class to handle processing of tokens."""
  def __call__( self, tokCount, tok ):
    """ Common code for handling tokens."""
    tokCount += 1
    self.metrics['tokenCount'] = tokCount
    if not tok.type in [WS,INDENT,DEDENT,EMPTY,ENDMARKER]:
      prevTok = self.saveTok
      self.saveTok = tok
    self.context['blockNum'] = self.metrics.get('blockCount',0)
    self.context['blockDepth'] = self.metrics.get('blockDepth',0)
    self.context['parenDepth'] = self.parenDepth
    self.context['bracketDepth'] = self.bracketDepth
    self.context['braceDepth'] = self.braceDepth

    self.classDepth += self.classDepthIncr
    self.context['classDepth'] = self.classDepth
    self.classDepthIncr = 0
    
    self.fcnDepth += self.fcnDepthIncr    # incr at start of fcn body
    self.context['fcnDepth'] = self.fcnDepth
    self.fcnDepthIncr = 0
    
    # start testing for types that change in context
    
    if self.docString and tok.type == token.STRING:
      self.docString = False
      tok.semtype = DOCSTRING
      if self.checkForModuleDocString:  # found the module's doc string
        self.metrics['numModuleDocStrings'] += 1
        self.checkForModuleDocString = False
      self.__postToken( tok )
      return
    if tok.type == COMMENT:
      # compensate for older tokenize including newline
      # obals......in token when only thing on line is comment
      # this patch makes all connents consistent
      self.metrics['numComments'] += 1
      if tok.text[-1] == '\n':
        tok.text = tok.text[:-1]
      if prevTok and prevTok.type != NEWLINE and prevTok.type != EMPTY:
        tok.semtype = INLINE
        self.metrics['numCommentsInline'] += 1
      self.__postToken( tok )
      return
      
    if self.defFunction:
      if tok.type == NAME:
        self.__incr( 'numFunctions')
        self.fqnName.append( tok.text )
        self.currentFcn = self.__genFQN( self.fqnName )
        self.fqnFunction.append( (self.currentFcn,self.blockDepth,FCNNAME) )
        self.defFunction = False
        self.findFcnHdrEnd = True
        if self.pa.verbose > 0: print "fqnFunction=%s" % self.fqnFunction
        self.__postToken( tok )
        return
      elif tok.type == ERRORTOKEN:
        # we must compensate for the simple scanner mishandling errors
        self.findFcnHdrEnd = True
        self.invalidToken = mytoken.MyToken(type=NAME, 
                                            semtype=FCNNAME, 
                                            text=tok.text, 
                                            row=tok.row, 
                                            col=tok.col, 
                                            line=tok.line)
        self.skipUntil = '(:\n'
        return
    elif self.defClass and tok.type == NAME:
      self.__incr( 'numClasses' )
      self.fqnName.append( tok.text )
      self.currentClass = self.__genFQN( self.fqnName )
      self.fqnClass.append( (self.currentClass,self.blockDepth,CLASSNAME) )
      self.defClass = False
      self.findClassHdrEnd = True
      if self.pa.verbose > 0: print "fqnClass=%s" % self.fqnClass
      self.__postToken( tok )
      return
      
    # return with types that don't change in context
    
    self.__postToken( tok )
    if tok.type == EMPTY:
      self.endOfStmt()
      return
    if tok.type == WS:
      return
    if tok.type == NEWLINE:
      self.endOfStmt()
      return
    
    # at this point, we have encountered a non-white space token
    # if a module doc string has not been found yet, it never will be.
    numModDocStrings = self.metrics['numModuleDocStrings']
    if self.checkForModuleDocString and numModDocStrings == 0:
      self.checkForModuleDocString = False
      msg = ( "Module %s is missing a module doc string. Detected at line %d"
               % (self.context['inFile'],tok.row) )
      if not self.pa.quietSw:
        sys.stderr.writelines( msg )

    if tok.type == OP and tok.text == ':':
      if self.findFcnHdrEnd:
        self.findFcnHdrEnd = False
        self.docString = True
        self.fcnDepthIncr = 1
        return
      elif self.findClassHdrEnd:
        self.findClassHdrEnd = False
        self.docString = True
        self.classDepthIncr = 1
        return
    if tok.type == OP:
      if tok.text == '(':
        self.parenDepth += 1
      elif tok.text == ')':
        self.parenDepth -= 1
      elif tok.text == '[':
        self.bracketDepth += 1
      elif tok.text == ']':
        self.bracketDepth -= 1
      elif tok.text == '{':
        self.braceDepth += 1
      elif tok.text == '}':
        self.braceDepth -= 1
      return
        
    if tok.type == INDENT:
      self.__incr( 'blockCount')
      self.blockDepth += 1
      self.metrics['blockDepth'] = self.blockDepth
      if self.metrics.get('maxBlockDepth',0) < self.blockDepth:
        self.metrics['maxBlockDepth'] = self.blockDepth
      return
    elif tok.type == DEDENT:
      assert self.fcnDepth == len( self.fqnFunction )
      self.blockDepth -= 1
      self.metrics['blockDepth'] = self.blockDepth
      while len(self.fqnFunction) and self.blockDepth<=self.fqnFunction[-1][1]:
        msg = self.endOfFunction( self.fcnExits )
        if msg and not self.pa.quietSw:
          print msg
        if self.pa.verbose > 0: 
          print "Removing function %s" % self.__extractFQN( self.fqnFunction )
        del self.fqnFunction[-1]
        del self.fqnName[-1]
        self.fcnDepth -= 1
      assert self.fcnDepth == len( self.fqnFunction )
      if len( self.fqnFunction ) > 0:
        self.currentFcn = self.fqnFunction[-1][0]
      else:
        self.currentFcn = None
      while len(self.fqnClass) and self.blockDepth <= self.fqnClass[-1][1]:
        self.endOfClass()
        self.classDepth -= 1
        if self.pa.verbose > 0: 
          print "Removing class %s" % self.__extractFQN( self.fqnClass )
        del self.fqnClass[-1]
        del self.fqnName[-1]
      if len( self.fqnClass ) > 0:
        self.currentClass = self.fqnClass[-1][0]
      else:
        self.currentClass = None
      return

    self.docString = False
    if tok.semtype == KEYWORD:
      self.__incr( 'numKeywords')
      if tok.text == 'def':
        self.defFunction = True
      elif tok.text == 'class':
        self.defClass = True
      elif tok.text == 'return':
        assert self.fcnDepth == len( self.fqnFunction )
        if self.fcnDepth == 0:     # then we are not in any function
          if not self.pa.quietSw:  # then report on these types of errors
            msg = ("Module %s contains the return statement" +
                   " at line %d that is outside any function" % 
                   (self.context['inFile'],tok.row) )
            print msg
        if prevTok.text == ':': 
          # this return on same line as conditional, 
          # so it must be an extra return
          self.fcnExits.append( tok.row )
        elif self.blockDepth > 1:
          # Let fcnBlockDepth be the block depth of the function body.
          # We are trying to count the number of return statements
          # in this function. Only one is allowed at the fcnBlockDepth for the
          # function. If the self.blockDepth is greater than fcnBlockDepth, 
          # then this is a conditional return - i.e., an additional return
          fcnBlockDepth = self.fqnFunction[-1][1] + 1
          if self.blockDepth > fcnBlockDepth:
            self.fcnExits.append( tok.row )
            
    return

To produce Listing 8, I first highlighted lines 35-54 in Listing 7 and then used the
Extract Method wizard to move them into the doString() method.
Listing 8. Moving Code with the Extract
Method Wizard

""" TokenHandler readies tokens to passing to metric routines.

  $Id: tokenhandler.py,v 1.2 2005/01/19 06:13:33 rcharney Exp $
"""
__version__ = "$Revision: 1.2 $"[11:-2]

#import token
from globals import *

class TokenHandler( object ):
  """ Class to handle processing of tokens."""
  def __call__( self, tokCount, tok ):
    """ Common code for handling tokens."""
    tokCount += 1
    self.metrics['tokenCount'] = tokCount
    if not tok.type in [WS,INDENT,DEDENT,EMPTY,ENDMARKER]:
      prevTok = self.saveTok
      self.saveTok = tok
    self.context['blockNum'] = self.metrics.get('blockCount',0)
    self.context['blockDepth'] = self.metrics.get('blockDepth',0)
    self.context['parenDepth'] = self.parenDepth
    self.context['bracketDepth'] = self.bracketDepth
    self.context['braceDepth'] = self.braceDepth

    self.classDepth += self.classDepthIncr
    self.context['classDepth'] = self.classDepth
    self.classDepthIncr = 0
    
    self.fcnDepth += self.fcnDepthIncr    # incr at start of fcn body
    self.context['fcnDepth'] = self.fcnDepth
    self.fcnDepthIncr = 0
    
    # start testing for types that change in context
    
    self.doStrings(tok, prevTok)
      
    if self.defFunction:
      if tok.type == NAME:
        self.__incr( 'numFunctions')
        self.fqnName.append( tok.text )
        self.currentFcn = self.__genFQN( self.fqnName )
        self.fqnFunction.append( (self.currentFcn,self.blockDepth,FCNNAME) )
        self.defFunction = False
        self.findFcnHdrEnd = True
        if self.pa.verbose > 0: print "fqnFunction=%s" % self.fqnFunction
        self.__postToken( tok )
        return
      elif tok.type == ERRORTOKEN:
        # we must compensate for the simple scanner mishandling errors
        self.findFcnHdrEnd = True
        self.invalidToken = mytoken.MyToken(type=NAME, 
                                            semtype=FCNNAME, 
                                            text=tok.text, 
                                            row=tok.row, 
                                            col=tok.col, 
                                            line=tok.line)
        self.skipUntil = '(:\n'
        return
    elif self.defClass and tok.type == NAME:
      self.__incr( 'numClasses' )
      self.fqnName.append( tok.text )
      self.currentClass = self.__genFQN( self.fqnName )
      self.fqnClass.append( (self.currentClass,self.blockDepth,CLASSNAME) )
      self.defClass = False
      self.findClassHdrEnd = True
      if self.pa.verbose > 0: print "fqnClass=%s" % self.fqnClass
      self.__postToken( tok )
      return
      
    # return with types that don't change in context
    
    self.__postToken( tok )
    if tok.type == EMPTY:
      self.endOfStmt()
      return
    if tok.type == WS:
      return
    if tok.type == NEWLINE:
      self.endOfStmt()
      return
    
    # at this point, we have encountered a non-white space token
    # if a module doc string has not been found yet, it never will be.
    numModDocStrings = self.metrics['numModuleDocStrings']
    if self.checkForModuleDocString and numModDocStrings == 0:
      self.checkForModuleDocString = False
      msg = ( "Module %s is missing a module doc string. Detected at line %d" 
              % (self.context['inFile'],tok.row) )
      if not self.pa.quietSw:
        sys.stderr.writelines( msg )

    if tok.type == OP and tok.text == ':':
      if self.findFcnHdrEnd:
        self.findFcnHdrEnd = False
        self.docString = True
        self.fcnDepthIncr = 1
        return
      elif self.findClassHdrEnd:
        self.findClassHdrEnd = False
        self.docString = True
        self.classDepthIncr = 1
        return
    if tok.type == OP:
      if tok.text == '(':
        self.parenDepth += 1
      elif tok.text == ')':
        self.parenDepth -= 1
      elif tok.text == '[':
        self.bracketDepth += 1
      elif tok.text == ']':
        self.bracketDepth -= 1
      elif tok.text == '{':
        self.braceDepth += 1
      elif tok.text == '}':
        self.braceDepth -= 1
      return
        
    if tok.type == INDENT:
      self.__incr( 'blockCount')
      self.blockDepth += 1
      self.metrics['blockDepth'] = self.blockDepth
      if self.metrics.get('maxBlockDepth',0) < self.blockDepth:
        self.metrics['maxBlockDepth'] = self.blockDepth
      return
    elif tok.type == DEDENT:
      assert self.fcnDepth == len( self.fqnFunction )
      self.blockDepth -= 1
      self.metrics['blockDepth'] = self.blockDepth
      while len(self.fqnFunction) and self.blockDepth<=self.fqnFunction[-1][1]:
        msg = self.endOfFunction( self.fcnExits )
        if msg and not self.pa.quietSw:
          print msg
        if self.pa.verbose > 0: 
          print "Removing function %s" % self.__extractFQN( self.fqnFunction )
        del self.fqnFunction[-1]
        del self.fqnName[-1]
        self.fcnDepth -= 1
      assert self.fcnDepth == len( self.fqnFunction )
      if len( self.fqnFunction ) > 0:
        self.currentFcn = self.fqnFunction[-1][0]
      else:
        self.currentFcn = None
      while len(self.fqnClass) and self.blockDepth <= self.fqnClass[-1][1]:
        self.endOfClass()
        self.classDepth -= 1
        if self.pa.verbose > 0: 
          print "Removing class %s" % self.__extractFQN( self.fqnClass )
        del self.fqnClass[-1]
        del self.fqnName[-1]
      if len( self.fqnClass ) > 0:
        self.currentClass = self.fqnClass[-1][0]
      else:
        self.currentClass = None
      return

    self.docString = False
    if tok.semtype == KEYWORD:
      self.__incr( 'numKeywords')
      if tok.text == 'def':
        self.defFunction = True
      elif tok.text == 'class':
        self.defClass = True
      elif tok.text == 'return':
        assert self.fcnDepth == len( self.fqnFunction )
        if self.fcnDepth == 0:     # then we are not in any function
          if not self.pa.quietSw:  # then report on these types of errors
            msg = ( "Module %s contains the return statement at line %d " +
                    "that is outside any function" 
                    % (self.context['inFile'],tok.row) )
        if prevTok.text == ':': 
          # this return on same line as conditional, 
          # so it must be an extra return
          self.fcnExits.append( tok.row )
        elif self.blockDepth > 1:
          # Let fcnBlockDepth be the block depth of the function body.
          # We are trying to count the number of return statements
          # in this function. Only one is allowed at the fcnBlockDepth for the
          # function. If the self.blockDepth is greater than fcnBlockDepth, 
          # then this is a conditional return - i.e., an additional return
          fcnBlockDepth = self.fqnFunction[-1][1] + 1
          if self.blockDepth > fcnBlockDepth:
            self.fcnExits.append( tok.row )
            
    return

  def doStrings(self, tok, prevTok):
    if self.docString and tok.type == token.STRING:
      self.docString = False
      tok.semtype = DOCSTRING
      if self.checkForModuleDocString:  # we have found the module's doc string
        self.metrics['numModuleDocStrings'] += 1
        self.checkForModuleDocString = False
      self.__postToken( tok )
      return
    if tok.type == COMMENT:
      # compensate for older tokenize including newline
      # obals......in token when only thing on line is comment
      # this patch makes all connents consistent
      self.metrics['numComments'] += 1
      if tok.text[-1] == '\n':
        tok.text = tok.text[:-1]
      if prevTok and prevTok.type != NEWLINE and prevTok.type != EMPTY:
        tok.semtype = INLINE
        self.metrics['numCommentsInline'] += 1
      self.__postToken( tok )
      return

Notice that BRM is not as clever as Eclipse. It did not detect the use of
return statements; it simply moved them into
the extracted method. This would cause a problem when running, because the return would
be returning from the extracted method--not from the original __call__
method. I fixed this manually by modifying the new method to return True
if one of its returns was used; otherwise, it returned False. The __call__
method then tested the return value and exited itself if True. I also
realized that the method name doStrings was a slight misnomer because
the method also handled comments. So, I changed the method name to
doStringsOrComments using BRM's Rename feature. Listing 9 shows these
changes.
Listing 9. Renaming with BRM's Rename
Feature

""" TokenHandler readies tokens to passing to metric routines.

  $Id: tokenhandler.py,v 1.2 2005/01/19 06:13:33 rcharney Exp $
"""
__version__ = "$Revision: 1.2 $"[11:-2]

#import token
from globals import *

class TokenHandler( object ):
  """ Class to handle processing of tokens."""
  def __call__( self, tokCount, tok ):
    """ Common code for handling tokens."""
    tokCount += 1
    self.metrics['tokenCount'] = tokCount
    if not tok.type in [WS,INDENT,DEDENT,EMPTY,ENDMARKER]:
      prevTok = self.saveTok
      self.saveTok = tok
    self.context['blockNum'] = self.metrics.get('blockCount',0)
    self.context['blockDepth'] = self.metrics.get('blockDepth',0)
    self.context['parenDepth'] = self.parenDepth
    self.context['bracketDepth'] = self.bracketDepth
    self.context['braceDepth'] = self.braceDepth

    self.classDepth += self.classDepthIncr
    self.context['classDepth'] = self.classDepth
    self.classDepthIncr = 0
    
    self.fcnDepth += self.fcnDepthIncr    # incr at start of fcn body
    self.context['fcnDepth'] = self.fcnDepth
    self.fcnDepthIncr = 0
    
    # start testing for types that change in context
    
    if self.doStringsOrComments(tok, prevTok):
      return
      
    if self.doFunctionName(tok):
      return      
    elif self.doClassName(tok):
      return
      
    # return with types that don't change in context
    
    self.__postToken( tok )
    if tok.type == EMPTY:
      self.endOfStmt()
      return
    if tok.type == WS:
      return
    if tok.type == NEWLINE:
      self.endOfStmt()
      return
    
    # at this point, we have encountered a non-white space token
    # if a module doc string has not been found yet, it never will be.
    numModDocStrings = self.metrics['numModuleDocStrings']
    if self.checkForModuleDocString and numModDocStrings == 0:
      self.checkForModuleDocString = False
      msg = ( "Module %s is missing a module doc string. Detected at line %d" 
              % (self.context['inFile'],tok.row) )
      print msg
      if not self.pa.quietSw:
        sys.stderr.writelines( msg )

    if tok.type == OP and tok.text == ':':
      if self.findFcnHdrEnd:
        self.findFcnHdrEnd = False
        self.docString = True
        self.fcnDepthIncr = 1
        return
      elif self.findClassHdrEnd:
        self.findClassHdrEnd = False
        self.docString = True
        self.classDepthIncr = 1
        return
    if tok.type == OP:
      if tok.text == '(':
        self.parenDepth += 1
      elif tok.text == ')':
        self.parenDepth -= 1
      elif tok.text == '[':
        self.bracketDepth += 1
      elif tok.text == ']':
        self.bracketDepth -= 1
      elif tok.text == '{':
        self.braceDepth += 1
      elif tok.text == '}':
        self.braceDepth -= 1
      return
        
    if tok.type == INDENT:
      self.__incr( 'blockCount')
      self.blockDepth += 1
      self.metrics['blockDepth'] = self.blockDepth
      if self.metrics.get('maxBlockDepth',0) < self.blockDepth:
        self.metrics['maxBlockDepth'] = self.blockDepth
      return
    elif tok.type == DEDENT:
      assert self.fcnDepth == len( self.fqnFunction )
      self.blockDepth -= 1
      self.metrics['blockDepth'] = self.blockDepth
      while len(self.fqnFunction) and self.blockDepth<=self.fqnFunction[-1][1]:
        msg = self.endOfFunction( self.fcnExits )
        if msg and not self.pa.quietSw:
          print msg
        if self.pa.verbose > 0: 
          print "Removing function %s" % self.__extractFQN( self.fqnFunction )
        del self.fqnFunction[-1]
        del self.fqnName[-1]
        self.fcnDepth -= 1
      assert self.fcnDepth == len( self.fqnFunction )
      if len( self.fqnFunction ) > 0:
        self.currentFcn = self.fqnFunction[-1][0]
      else:
        self.currentFcn = None
      while len(self.fqnClass) and self.blockDepth <= self.fqnClass[-1][1]:
        self.endOfClass()
        self.classDepth -= 1
        if self.pa.verbose > 0: 
          print "Removing class %s" % self.__extractFQN( self.fqnClass )
        del self.fqnClass[-1]
        del self.fqnName[-1]
      if len( self.fqnClass ) > 0:
        self.currentClass = self.fqnClass[-1][0]
      else:
        self.currentClass = None
      return

    self.docString = False
    if tok.semtype == KEYWORD:
      self.__incr( 'numKeywords')
      if tok.text == 'def':
        self.defFunction = True
      elif tok.text == 'class':
        self.defClass = True
      elif tok.text == 'return':
        assert self.fcnDepth == len( self.fqnFunction )
        if self.fcnDepth == 0:     # then we are not in any function
          if not self.pa.quietSw:  # then report on these types of errors
            msg = ( "Module %s contains the return statement at line %d " +
                    "that is outside any function" % 
                    (self.context['inFile'],tok.row) )
        if prevTok.text == ':': 
          # this return on same line as conditional, 
          # so it must be an extra return
          self.fcnExits.append( tok.row )
        elif self.blockDepth > 1:
          # Let fcnBlockDepth be the block depth of the function body.
          # We are trying to count the number of return statements
          # in this function. Only one is allowed at the fcnBlockDepth for the
          # function. If the self.blockDepth is greater than fcnBlockDepth, 
          # then this is a conditional return - i.e., an additional return
          fcnBlockDepth = self.fqnFunction[-1][1] + 1
          if self.blockDepth > fcnBlockDepth:
            self.fcnExits.append( tok.row )
            
    return

  def doClassName(self, tok):
    """ Return True if current token is class name."""
    if self.defClass and tok.type == NAME:
      self.__incr( 'numClasses' )
      self.fqnName.append( tok.text )
      self.currentClass = self.__genFQN( self.fqnName )
      self.fqnClass.append( (self.currentClass,self.blockDepth,CLASSNAME) )
      self.defClass = False
      self.findClassHdrEnd = True
      if self.pa.verbose > 0: print "fqnClass=%s" % self.fqnClass
      self.__postToken( tok )
      return True
    return False

  def doFunctionName(self, tok):
    """ Return True if this is a function name."""
    if self.defFunction:
      if tok.type == NAME:
        self.__incr( 'numFunctions')
        self.fqnName.append( tok.text )
        self.currentFcn = self.__genFQN( self.fqnName )
        self.fqnFunction.append( (self.currentFcn,self.blockDepth,FCNNAME) )
        self.defFunction = False
        self.findFcnHdrEnd = True
        if self.pa.verbose > 0: print "fqnFunction=%s" % self.fqnFunction
        self.__postToken( tok )
        return True
      elif tok.type == ERRORTOKEN:
        # we must compensate for the simple scanner mishandling errors
        self.findFcnHdrEnd = True
        self.invalidToken = mytoken.MyToken(type=NAME, 
                                            semtype=FCNNAME, 
                                            text=tok.text, 
                                            row=tok.row, 
                                            col=tok.col, 
                                            line=tok.line)
        self.skipUntil = '(:\n'
        return True
      return False

  def doStringsOrComments(self, tok, prevTok):
    """ Return True if this is a docstring or comment."""
    if self.docString and tok.type == token.STRING:
      self.docString = False
      tok.semtype = DOCSTRING
      if self.checkForModuleDocString:  # we have found the module's doc string
        self.metrics['numModuleDocStrings'] += 1
        self.checkForModuleDocString = False
      self.__postToken( tok )
      return True
    if tok.type == COMMENT:
      # compensate for older tokenize including newline
      # obals......in token when only thing on line is comment
      # this patch makes all connents consistent
      self.metrics['numComments'] += 1
      if tok.text[-1] == '\n':
        tok.text = tok.text[:-1]
      if prevTok and prevTok.type != NEWLINE and prevTok.type != EMPTY:
        tok.semtype = INLINE
        self.metrics['numCommentsInline'] += 1
      self.__postToken( tok )
      return True
    return False

An early refactoring program, BRM has a number of other issues of which you
should be aware. They include:

• If a variable is used before an extracted method and
  later is redefined in the extracted method, the variable is passed as an
  argument to the extracted method, even when the passed value is never
  used.
• If an argument is a simple variable and it is modified
  in the extracted method, the name in the extracted method is rebound to
  a different object. (For example, the simple parameter is passed as if it were
  a value.) This leaves unchanged the variable passed into the
  extracted method when the method returns.

Far more refactoring can be done on both the Java and Python code
samples. But, I believe I have given you a brief overview of what
current refactoring tools can do.
Conclusion
The benefits of using tools to do code refactoring include:

• They take a fair amount of drudgery out of refactoring
  code.
• They calculate which variables need to appear in the
  parameter list of the extracted methods, barring the problem
  above.
• They make renaming variables/classes/functions/methods
  almost painless.

I realize I haven't discussed why you should refactor your code, or anyone
else's for that matter. Often, there are both technical and managerial reasons to do
refactoring. Using prototypes for production purposes is one of the
bugaboos of programming, and we know the problems this causes. However, refactoring may
be a way out of this cul-de-sac. Perhaps, you can use the benefits of
refactoring to justify a rewrite to management.
Postscript
I am puzzled by the differences in the interest between refactoring and
metrics. I have found no objective way of measuring the effect of code
refactoring. It seems that we simply know that the refactored code is
"better".

I am also bothered by my response to one comment on last month's
column. The writer mentioned that Function Points are an accepted
metric for measuring complexity. They also provide good, consistent
metrics for estimating project costs. I made an error of omission in
not mentioning function points. On further reflection, I still don't
see them as effective code metrics. Function points most often are used
during specification and design of systems. They also are labor-intensive.
I know of no product out there that can do a proper function point analysis
on an existing code base. Please correct me if I am wrong.
Resources
Sources for Information about
Refactoring

Open Source
Plugin for jEdit

An Older Java
Refactoring Tool

Java
freeware, usable as standalone or plugin for a number of editors and
IDEs; recent and extensive.

Pymetrics Home
Page

Refactoring: Improving the Design of Existing Code
by Martin Fowler, Don Roberts, John Brant, William Opdyke (ISBN
0-201-48567-2), Addison-Wesley.

Refactoring to Patterns by Joshua Kerievsky, ISBN 0-321-21335-1, Addison-Wesley