Programming Tools: Refactoring

A look at two open-source refactoring tools, Eclipse for Java and Bicycle Repair Man for Python.
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.

______________________

Comments

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.

Bicycle Repair Man and Eclipse

JLawhead's picture

It is also worth mentioning that Bicycle Repair Man is integrated into Eclipse through the excellent PyDev plug-in: http://pydev.sourceforge.net/

PyDev tries to offer all the great relevant tools available when working in Java to Python developers.

I also agree with the author that while the definition of refactoring can be abstracted to a very high architecture level most programmers use the term "refactor" to mean removing all of the hacks and shortcuts in code that were inserted because of a short timeline for a software demonstration, release, or for testing with components created by other developers on a team.

Bicycle Repair Man and Eclipse

JLawhead's picture

It is also worth mentioning that Bicycle Repair Man is integrated into Eclipse through the excellent PyDev plug-in: http://pydev.sourceforge.net/

PyDev tries to offer all the great relevant tools available when working in Java to Python developers.

I also agree with the author that while the definition of refactoring can be abstracted to a very high architecture level most programmers use the term "refactor" to mean removing all of the hacks and shortcuts in code that were inserted because of a short timeline for a software demonstration, release, or for testing with components created by other developers on a team.

Definition of refactoring

Dan Razzell's picture

Here is another instance where I think computer science is at risk of being obscured instead of aided by technology.

The article addresses refactoring as if it were concerned exclusively with code. It's not. Refactoring is primarily about architecture and only secondarily about code. In other words, the focus is on design more than implementation.

Peter Deutsch says, and I agree, that:

Interface design and functional factoring constitute the key intellectual content of software and are far more difficult to create or re-create than code.

Most of the work of refactoring a system consists of revising the relationships between its functional components, indeed adding and eliminating components as well. These are changes to structure, not details of implementation.

It needn't be a software system. It would be perfectly reasonable to refactor an automobile engine, for example, so that the timing chain might be driven off the clutch assembly rather than the generator assembly, or in a more extreme case the cylinder layout may change from inline to vee. In a broad sense, the engine is functionally unchanged. That is, it still meets its original design requirements. However, its structure may have changed radically.

When you refactor a system, you're taking a fresh look at its design tradeoffs. It's really a "big picture" exercise in which basic assumptions of modularity, flow of control, and data abstraction are reexamined. This may answer the author's misgivings about how we're supposed to "know that the refactored code is 'better'." It wouldn't be the code that's better but the design, and of course that is a matter best debated at the design level.

What then is the benefit of "refactoring" tools, since indeed many only pertain to code and not to design? Well, to belabor the engine analogy, a boring machine would be in a comparable sense an "engine refactoring" tool. It doesn't actually do any refactoring itself, but it allows you a way to implement the design changes which are the result of your refactoring efforts.

Definition of refactoring

Anonymous's picture

Your point is well taken. However, this column is about programming tools. My inclination is to be as concrete as I can. Since the title of this column is programming as opposed to design, and tools as in software that I can use to improve my programming experience, I chose to use the much narrower definition.

That said, if you are aware of any refactoring tools for the design portion of a system's life cycle, please let me know. I would be really interested.

Reg. Charney

Definition of refactoring

Dan Razzell's picture

I think your best bet for that approach would be to look at UML refactoring tools. A good overview can be found in Martin Fowler, Refactoring: Improving the Design of Existing Code.

This is not an endorsement of UML as the basis of refactoring, by the way, because I believe that misses the point. Refactoring is substantially a conceptual exercise. Like optimization, the biggest gains require an understanding of the problem domain. On the other hand, if UML is already part of your process, or if you're willing to make it one, then refactoring at that level would be completely appropriate. It certainly gets you closer to understanding the design issues of a project than if you were to go straight to the code.

Again, you have a valid point

R. Charney's picture

Again, you have a valid point. Do you or do any of our readers know of tools that would work on the UML description of a system?

Reg. Charney

M-x tags-query-replace

Anonymous's picture

enough said :-)

Webinar
One Click, Universal Protection: Implementing Centralized Security Policies on Linux Systems

As Linux continues to play an ever increasing role in corporate data centers and institutions, ensuring the integrity and protection of these systems must be a priority. With 60% of the world's websites and an increasing share of organization's mission-critical workloads running on Linux, failing to stop malware and other advanced threats on Linux can increasingly impact an organization's reputation and bottom line.

Learn More

Sponsored by Bit9

Webinar
Linux Backup and Recovery Webinar

Most companies incorporate backup procedures for critical data, which can be restored quickly if a loss occurs. However, fewer companies are prepared for catastrophic system failures, in which they lose all data, the entire operating system, applications, settings, patches and more, reducing their system(s) to “bare metal.” After all, before data can be restored to a system, there must be a system to restore it to.

In this one hour webinar, learn how to enhance your existing backup strategies for better disaster recovery preparedness using Storix System Backup Administrator (SBAdmin), a highly flexible bare-metal recovery solution for UNIX and Linux systems.

Learn More

Sponsored by Storix