Session 12

The Refactoring Process


810:188

Agile Software Development


Agile Moments

Explaining Refactoring

On the Yahoo! XP mailing list Monday, Kent Beck wrote:

> When my customer asks me why I bother improving the quality of the code I work
> on, I just tell them that that's the way we do it. I'm not going to negotiate
> quality with them -- me wanting it, and them wanting to cut it. That just makes
> no sense...

The customer has the right to ask the question and receive an understandable answer. It sounds ... like you already have the answer, you're just not giving it to the customer.

Customer: "Why are you spending time cleaning up your code?"

Me: "That's how we get the results you hired us to achieve. With clean code, as the project progresses we can give you good things more and more quickly rather than more and more slowly."

Your customer isn't too stupid to understand the value of process. It's your responsibility to explain it clearly. If you know in your heart the value of what you do, that will come through. Valuable ideas can withstand scrutiny.

I like that Kent "walks the walk" on the values that underlie XP and the agile methods. He encourages respect for all stakeholders and a lack of arrogance in how we behave. You'd think that was a part of being a professional, but so much of the computing industry trades on the gap between technical and non-technical people. Rather than acquiesce or cynically use the gap, perhaps instead we should try to educate.

On Outsourcing

Also on the Yahoo! XP mailing list Monday, Chet Hendrickson gave an agile perspective on a hot topic. The italics are mine.

> Just wondering what the programmers on this forum really think about outsourcing?

An awful lot of companies have IT organizations that deliver late, over budget, or not at all. They think that they can get at least the same results for a lot less by outsourcing.

If you are going to fail, doing so for a lot less may make sense.

One way we could "protect our jobs" is to do them better.

In the same vein, William Pietri made an interesting analogy to natural selection later in the same thread.


Exercise: Debriefing Release 1, Iteration 1

In order to get better at something, you have to do it. But you also need to assess your performance and adjust how you do it. So let's take just a few minutes to reflect on Iteration 1 from our Release 1 plan.

Get together with your project team and discuss the following items. Be as honest as you can -- it's hard to get better when you aren't looking at the world the way it really is.

[ discuss ]


Where Are We?

Last time, we looked at the code for a simple Tic-Tac-Toe GUI framework.

Quick Exercise: Review the code now and identify the absolute worst smell in this code. Ask yourself, "What is the #1 thing I want to see improved in this program before we go home today?"

And "It's written in Java." is not a satisfactory answer.

Save those suggestions...

As we've been discussing, refactoring is changing the internal structure of a program without changing its external behavior. We refactor primarily to improve the design of the code. We don't want this process to be random or willy-nilly, but rather the disciplined application of a series of small behavior preserving transformations.

Refactoring is as old as programming, but we've only been talking about it as a separate part of the development process for 25 years or so, and it has been a part of computing vernacular for a decade. Martin Fowler's book Refactoring: Improving the Design of Existing Code cemented the term in the popular mindset. The best refactoring resource on the web is the Refactoring Home Page, which provides links to tons of refactoring resources, including all of the refactorings from his book plus many new ones.

TDD implies the need to refactor. Refactoring implies the need for at least tests, if not TDD.


The Mechanics of Refactoring

Refactoring should consist of three steps:

  1. Recognize a smell.
  2. Make the smell go away.
  3. Demonstrate that you haven't broken anything.

Let's consider each in turn.


Recognizing Code Smells

Return to student suggestions for the absolute worst smell in the Tic-Tac-Toe code.

Collect those suggestions...

A code smell is something you notice when you read code. It doesn't mean that something is necessarily wrong, but it is a sign that something might be wrong.

Some smells can occur in any sort of code. These include:

Others depend on the style of programming you are doing. Some code smells in OO programs include:

Finally, some smells are even language-specific. For example, in Java, too much string addition is a bad sign, because Java's treatment of Strings can be rather inefficient.

Notice that in doing TDD you will often create code knowing that it smells, because that's the simplest thing you can do to make the code pass a test. Copy-and-paste duplication often occurs this way. No problem -- you know that you'll be refactoring the code soon, and this smell will still be fresh in your mind.

Recently, I wrote a little article that talks about the many ways in which duplication can occur in a program. It accounts for a lot of smells!


Eliminating Code Smells

Making code smella go away can't be that hard, right? It's just programming... Let's take a look at a sample from the above lists, and see what we might do.

General Smells

Consider this long method from the Cell class in our Tic-Tac-Toe code:

    public void paint(Graphics g) {
        int size = cell.width;
        int x = cell.x;
        int y = cell.y;
        switch (value) {
        case OFF: 
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            break;
        case ON:  
            g.setColor(color);
            g.fillRect(x, y, size, size);
            break;
        case EX:  
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            g.drawLine(x, y, x+size, y+size);
            g.drawLine(x+size, y, x, y+size);
            break;
        case OH:  
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            g.drawOval(x+1, y+1, size-2, size-2);
            break;
        } // switch
        g.setColor(Color.black);
        g.drawLine(x, y, x+size, y);
        g.drawLine(x, y+size, x+size, y+size);
        g.drawLine(x, y, x, y+size);
        g.drawLine(x+size, y, x+size, y+size);
    } // paint

Methods this long tend to be hard to understand because they are doing so many different things. Oftentimes, their length hides a meaningful operationin a mass of details. For example, that big switch must be doing something meaningful -- but what? It is drawing something in Cell's region based on its state.

One way to tame a long method is to extract a method from the code.

    public void paint(Graphics g) {
        int size = cell.width;
        int x = cell.x;
        int y = cell.y;

        paintCellState(g, x, y, size);

        g.setColor(Color.black);
        g.drawLine(x, y, x+size, y);
        g.drawLine(x, y+size, x+size, y+size);
        g.drawLine(x, y, x, y+size);
        g.drawLine(x+size, y, x+size, y+size);
    }

    public void paintCellState(Graphics g, int x, int y, int size) {
        switch (value) {
        case OFF: 
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            break;
        case ON:  
            g.setColor(color);
            g.fillRect(x, y, size, size);
            break;
        case EX:  
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            g.drawLine(x, y, x+size, y+size);
            g.drawLine(x+size, y, x, y+size);
            break;
        case OH:  
            g.setColor(Color.white);
            g.fillRect(x, y, size, size);
            g.setColor(color);
            g.drawOval(x+1, y+1, size-2, size-2);
            break;
        }
    }

Notice that by extracting the method, we are forced to name it, which automatically makes the original method easier to understand. Even better, the method name eliminates the need for one of those annoying comments that explains "this code paints the cell according to its state".

Extract Method is but one of many, many refactorings that are applicable to any kind of program. Add Parameter, Inline Method, Inline Temporary variable, and Replace Nested Conditional with Guard Clauses are a few others. See Fowler's catalog for even more.

OOP Smells

That new method still stinks. It uses a selection statement to decide how to behave. But in object-oriented programming, objects behave based on who they are. Programs use polymorphic dispatch to implement choices. How can we make this smell go away?

This isn't quite as simple as "just" extracting a method. But in agile fashion, we can make things simpler by breaking it into several smaller steps. (I know that breaking things into smaller steps is not an "agile thing", but the agile folks remind us to do it all the time!)

We can evolve this program into a better solution through a series of smaller refactorings:

Let's do that one later...

Java Smells

Each programming language has its idiosyncracies. Some manifest themselves in strange efficieny concerns. For example, Java Strings are immutable, that is, their values don't change. When you append two Strings, neither object is changed; a brand new String is created with the resulting value. But this this makes string operations rather expensive.

When we see code with a lot of String appends, we can refactor to a more efficient version that uses StringBuffers:

replace String with StringBuffer

Language-specific refactorings do not deal only with efficiency concerns. Often times, we create or come across code that violates a language's idioms, and we improve the code by changing the code to make it better meet the expectations of informed readers. In Java, these might involve looping style, placement of instance variables and methods within a class, and so on.

Summary

You may be thinking, "Those are all obvious!" Perhaps. But programmers continue to write code like this, either because it's the simplest thing that will work or because they don't know better. And some programmers are not sure what to do when they encounter the smells.

Many refactorings are obvious. Their targets are common design or coding patterns. (I think it's true that *all* refactorings target design or coding patterns, but some are not as common or obvious as others.) That's why learning the basics of good design and the common design patterns is essential to becoming a good programming -- even in an agile method, where we don't do Big Design Up Front.

Check out some of the refactoring resources I have posted for you, especially the refactorings catalog and some refactoring thumbnails.


Wrap Up


Eugene Wallingford ==== wallingf@cs.uni.edu ==== October 5, 2004