Lecture 26: Refactoring, Part 2

Credit where Credit is Due

Some of the material for this lecture is taken from “Refactoring: Improving the Design of Existing Code” by Martin Fowler; as such some material is copyright © Addison Wesley, 1999

Last Lecture

Refactoring
- Introduced core ideas
  - Improve structure of software system without changing functionality
  - Watch out for “bad smells” in code
  - Do refactoring in the presence of test cases; if a test fails, something went wrong with the previous refactoring
- Covered several examples

Goals of this Lecture

Present a tutorial using a larger example that requires multiple refactorings
Show how test cases can make refactoring safe
We saw a “graphical overview” of this tutorial at the end of lecture 24

Tutorial: Background Information

We have a (very) simple system for a video rental store
It contains three classes
- Movie
- Rental
- Customer
A customer keeps track of its rentals and can produce a statement (in ASCII) showing how much its rentals cost and how many “frequent rental points” they provide.
We'd like to modify the code to also be able to produce a statement formatted in HTML and have discovered that none of the existing code can be reused!
Refactoring to the rescue!

Does this system need refactoring?

For such a simple system: probably not
But imagine that these three classes are part of a larger system
- then the refactorings we do during the tutorial can indeed be useful
The point is to imagine following this process on a daily basis within the context of production software development
- refactoring needs to be incremental, systematic, and safe

Initial Class Diagram

Before

statement() works by looping through all rentals; for each rental, it retrieves its movie and the number of days it was rented; it also retrieves the price code of the movie

It then calculates the price for each movie rental and the number of frequent renter points and returns the generated statement as a string

Initial Code: Movie

public class Movie {

    public static final int CHILDRENS   = 2;
    public static final int REGULAR     = 0;
    public static final int NEW_RELEASE = 1;

    private String _title;
    private int    _priceCode;

    public Movie(String title, int priceCode) {
        _title = title;
        _priceCode = priceCode;
    }

    public int getPriceCode() {
        return _priceCode;
    }

    public void setPriceCode(int arg) {
        _priceCode = arg;
    }

    public String getTitle() {
        return _title;
    }
}

Initial Code: Rental

public class Rental {
    private Movie _movie;
    private int   _daysRented;

    public Rental(Movie movie, int daysRented) {
        _movie      = movie;
        _daysRented = daysRented;
    }

    public int getDaysRented() {
        return _daysRented;
    }

    public Movie getMovie() {
        return _movie;
    }
}

Initial Code: Customer

import java.util.Enumeration;
import java.util.Vector;

public class Customer {
    private String _name;
    private Vector _rentals = new Vector();

    public Customer (String name) {
        _name = name;
    }

    public void addRental(Rental arg) {
        _rentals.addElement(arg);
    }

    public String getName() {
        return _name;
    }

    public String statement() {

        double      totalAmount          = 0;
        int         frequentRenterPoints = 0;
        Enumeration rentals              = _rentals.elements();
        String      result               = "Rental Record for " + getName() + "\n";

        while (rentals.hasMoreElements()) {

            double thisAmount = 0;
            Rental each       = (Rental) rentals.nextElement();

            // determine amounts for each line
            switch (each.getMovie().getPriceCode()) {
                case Movie.REGULAR:
                    thisAmount += 2;
                    if (each.getDaysRented() > 2) {
                        thisAmount += (each.getDaysRented() - 2) * 1.5;
                    }
                    break;
                case Movie.NEW_RELEASE:
                    thisAmount += each.getDaysRented() * 3;
                    break;
                case Movie.CHILDRENS:
                    thisAmount += 1.5;
                    if (each.getDaysRented() > 3) {
                        thisAmount += (each.getDaysRented() - 3) * 1.5;
                    }
                    break;
            }

            // add frequent renter points
            frequentRenterPoints++;

            // add bonus for a two day new release rental
            if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
                (each.getDaysRented() > 1)) {
                    frequentRenterPoints++;
            }

            // show figures for this rental
            result += "\t" + each.getMovie().getTitle() +
                      "\t" + String.valueOf(thisAmount) + "\n";
            totalAmount += thisAmount;
        }

        // add footer lines
        result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
        result += "You earned " + String.valueOf(frequentRenterPoints) +
                  " frequent renter points";
        return result;
    }
}

Step 1: Refactor statement()

Why? Its a “long method”, one of the “bad smells” covered in the previous lecture
Also, recall that our purpose is to add a new method to Customer to generate an HTML statement
- refactoring statement() may lead to code that can be reused by this new method
This matches one of Fowler’s conditions for refactoring: cleaning up the code to make it possible to add a new function

But WAIT!!!

Where's our test case?
We need to have a test case before we can refactor the existing code!

Our Test Case

import java.io.*;

public class useCustomer {

    private static Movie monty = new Movie("Life of Brian", Movie.REGULAR);
    private static Movie terry = new Movie("Brazil", Movie.REGULAR);
    private static Movie disney = new Movie("Snow White", Movie.CHILDRENS);
    private static Movie nicole = new Movie("Moulin Rouge", Movie.NEW_RELEASE);

    private static Rental r1 = new Rental(monty, 7);
    private static Rental r2 = new Rental(terry, 14);
    private static Rental r3 = new Rental(disney, 4);
    private static Rental r4 = new Rental(nicole, 2);

    private static Customer ken = new Customer("Ken Anderson");

    public static void main(String[] args) {
        try {
            ken.addRental(r1);
            ken.addRental(r2);
            ken.addRental(r3);
            ken.addRental(r4);

            File target = new File(args[0]);

            LineNumberReader in = new LineNumberReader(new FileReader(target));

            String targetStatement = null;
            String line            = null;

            while ((line = in.readLine()) != null) {
                if (targetStatement != null) {
                    targetStatement += line;
                } else {
                    targetStatement = line;
                }
                targetStatement += "\n";
            }
            // strip off last \n
            targetStatement =
                targetStatement.substring(0, targetStatement.length()-1);

            System.out.println("Current Statement:\n\n" + ken.statement());
            System.out.println("");
            System.out.println("");
            System.out.println("Target Statement:\n\n" + targetStatement);
            System.out.println("");

            if (targetStatement.equals(ken.statement())) {
                System.out.println("Test Passed.");
            } else {
                System.out.println("Test Failed.");
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Our Test Statement

Rental Record for Ken Anderson
	Life of Brian	9.5
	Brazil	20.0
	Snow White	3.0
	Moulin Rouge	6.0
Amount owed is 38.5
You earned 5 frequent renter points

Note: Lines 2-5 have a single tab character both before and after the movie name

Test Run on Original Program

$ java -classpath . useCustomer targetStatement.txt 
Current Statement:

Rental Record for Ken Anderson
        Life of Brian   9.5
        Brazil  20.0
        Snow White      3.0
        Moulin Rouge    6.0
Amount owed is 38.5
You earned 5 frequent renter points


Target Statement:

Rental Record for Ken Anderson
        Life of Brian   9.5
        Brazil  20.0
        Snow White      3.0
        Moulin Rouge    6.0
Amount owed is 38.5
You earned 5 frequent renter points

Test Passed.

Back to Step 1

We want to decompose statement() into smaller pieces
We'll start with Extract Method and target the switch statement first

// determine amounts for each line
switch (each.getMovie().getPriceCode()) {
    case Movie.REGULAR:
        thisAmount += 2;
        if (each.getDaysRented() > 2) {
            thisAmount += (each.getDaysRented() - 2) * 1.5;
        }
        break;
    case Movie.NEW_RELEASE:
        thisAmount += each.getDaysRented() * 3;
        break;
    case Movie.CHILDRENS:
        thisAmount += 1.5;
        if (each.getDaysRented() > 3) {
            thisAmount += (each.getDaysRented() - 3) * 1.5;
        }
        break;
}

Extract Method tells us to look at variables local to the code being extracted
- unmodified variables can become parameters to the new method: each
- modified variables require more care: since we have only one, we'll make thisAmount the method's return value
The new method is below

private double amountFor(Rental each) {
    double thisAmount = 0;

    switch (each.getMovie().getPriceCode()) {
        case Movie.REGULAR:
            thisAmount += 2;
            if (each.getDaysRented() > 2) {
                thisAmount += (each.getDaysRented() - 2) * 1.5;
            }
            break;
        case Movie.NEW_RELEASE:
            thisAmount += each.getDaysRented() * 3;
            break;
        case Movie.CHILDRENS:
            thisAmount += 1.5;
            if (each.getDaysRented() > 3) {
                thisAmount += (each.getDaysRented() - 3) * 1.5;
            }
            break;
    }

    return thisAmount;
}

In its original location, statement() now has the following line of code

thisAmount = amountFor(each);

statement() is now smaller and we've gained a method that can be used by our planned htmlStatement() method

Step 1 in Detail: Original Code

public class Customer {
…
    public String statement() {

        double      totalAmount          = 0;
        int         frequentRenterPoints = 0;
        Enumeration rentals              = _rentals.elements();
        String      result               = "Rental Record for " + getName() + "\n";

        while (rentals.hasMoreElements()) {

            double thisAmount = 0;
            Rental each       = (Rental) rentals.nextElement();

            // determine amounts for each line
            switch (each.getMovie().getPriceCode()) {
                case Movie.REGULAR:
                    thisAmount += 2;
                    if (each.getDaysRented() > 2) {
                        thisAmount += (each.getDaysRented() - 2) * 1.5;
                    }
                    break;
                case Movie.NEW_RELEASE:
                    thisAmount += each.getDaysRented() * 3;
                    break;
                case Movie.CHILDRENS:
                    thisAmount += 1.5;
                    if (each.getDaysRented() > 3) {
                        thisAmount += (each.getDaysRented() - 3) * 1.5;
                    }
                    break;
            }

            // add frequent renter points
            frequentRenterPoints++;

            // add bonus for a two day new release rental
            if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
                (each.getDaysRented() > 1)) {
                    frequentRenterPoints++;
            }

            // show figures for this rental
            result += "\t" + each.getMovie().getTitle() +
                      "\t" + String.valueOf(thisAmount) + "\n";
            totalAmount += thisAmount;
        }

        // add footer lines
        result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
        result += "You earned " + String.valueOf(frequentRenterPoints) +
                  " frequent renter points";
        return result;
    }
…
}

Step 1 in Detail: Extract Method

public class Customer {
…
    public String statement() {

        double      totalAmount          = 0;
        int         frequentRenterPoints = 0;
        Enumeration rentals              = _rentals.elements();
        String      result               = "Rental Record for " + getName() + "\n";

        while (rentals.hasMoreElements()) {

            double thisAmount = 0;
            Rental each       = (Rental) rentals.nextElement();

            // determine amounts for each line
            switch (each.getMovie().getPriceCode()) {
                case Movie.REGULAR:
                    thisAmount += 2;
                    if (each.getDaysRented() > 2) {
                        thisAmount += (each.getDaysRented() - 2) * 1.5;
                    }
                    break;
                case Movie.NEW_RELEASE:
                    thisAmount += each.getDaysRented() * 3;
                    break;
                case Movie.CHILDRENS:
                    thisAmount += 1.5;
                    if (each.getDaysRented() > 3) {
                        thisAmount += (each.getDaysRented() - 3) * 1.5;
                    }
                    break;
            }

            // add frequent renter points
            frequentRenterPoints++;

            // add bonus for a two day new release rental
            if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
                (each.getDaysRented() > 1)) {
                    frequentRenterPoints++;
            }

            // show figures for this rental
            result += "\t" + each.getMovie().getTitle() +
                      "\t" + String.valueOf(thisAmount) + "\n";
            totalAmount += thisAmount;
        }

        // add footer lines
        result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
        result += "You earned " + String.valueOf(frequentRenterPoints) +
                  " frequent renter points";
        return result;
    }
    
    private double amountFor(Rental each) {
        double thisAmount = 0;

        switch (each.getMovie().getPriceCode()) {
            case Movie.REGULAR:
                thisAmount += 2;
                if (each.getDaysRented() > 2) {
                    thisAmount += (each.getDaysRented() - 2) * 1.5;
                }
                break;
            case Movie.NEW_RELEASE:
                thisAmount += each.getDaysRented() * 3;
                break;
            case Movie.CHILDRENS:
                thisAmount += 1.5;
                if (each.getDaysRented() > 3) {
                    thisAmount += (each.getDaysRented() - 3) * 1.5;
                }
                break;
        }

        return thisAmount;
    }
…

Step 1 in Detail: Update statement to call new method

public String statement() {

    double      totalAmount          = 0;
    int         frequentRenterPoints = 0;
    Enumeration rentals              = _rentals.elements();
    String      result               = &quot;Rental Record for &quot; + getName() + &quot;\n&quot;;

    while (rentals.hasMoreElements()) {

        double thisAmount = 0;
        Rental each       = (Rental) rentals.nextElement();

        // determine amounts for each line
        thisAmount = amountFor(each);

        // add frequent renter points
        frequentRenterPoints++;

        // add bonus for a two day new release rental
        if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &amp;&amp;
            (each.getDaysRented() &gt; 1)) {
                frequentRenterPoints++;
        }

        // show figures for this rental
        result += &quot;\t&quot; + each.getMovie().getTitle() +
                  &quot;\t&quot; + String.valueOf(thisAmount) + &quot;\n&quot;;
        totalAmount += thisAmount;
    }

    // add footer lines
    result += &quot;Amount owed is &quot; + String.valueOf(totalAmount) + &quot;\n&quot;;
    result += &quot;You earned &quot; + String.valueOf(frequentRenterPoints) +
              &quot; frequent renter points&quot;;
    return result;
}

Possible Pitfall

Note: in the previous example, the return value of our extracted method needed to be “double”
It would be easy to make a mistake in this situation and have the new method return an int
If we had, however, our test case would have caught it... here's the output of our test case, if we had made that mistake

$ java -classpath . useCustomer targetStatement.txt 
Current Statement:

Rental Record for Ken Anderson
        Life of Brian   9.0
        Brazil  20.0
        Snow White      2.0
        Moulin Rouge    6.0
Amount owed is 37.0
You earned 5 frequent renter points


Target Statement:

Rental Record for Ken Anderson
        Life of Brian   9.5
        Brazil  20.0
        Snow White      3.0
        Moulin Rouge    6.0
Amount owed is 38.5
You earned 5 frequent renter points

Test Failed.

This mistakes causes the test case to fail and we would have been warned that we had done something wrong during the refactoring

Step 2: Rename variables

The variable names in the new amountFor() method don't make sense now that they have been moved out of the statement() method
As Fowler says Any fool can write code that a computer can understand. Good programmers write code that humans can understand
Lets change each to aRental and thisAmount to result. The new method is below:

private double amountFor(Rental aRental) {
    double result = 0;

    switch (aRental.getMovie().getPriceCode()) {
        case Movie.REGULAR:
            result += 2;
            if (aRental.getDaysRented() > 2) {
                result += (aRental.getDaysRented() - 2) * 1.5;
            }
            break;
        case Movie.NEW_RELEASE:
            result += aRental.getDaysRented() * 3;
            break;
        case Movie.CHILDRENS:
            result += 1.5;
            if (aRental.getDaysRented() > 3) {
                result += (aRental.getDaysRented() - 3) * 1.5;
            }
            break;
    }

    return result;
}

We would now run our test case again to verify that the program still runs

Step 3: Move Method

The amountFor() method uses a lot of information from the Rental class
Furthermore, it does not use ANY information from the Customer class
Lets move this method to the Rental class
- We won't need the parameter anymore
- Lets also change the name from amountFor() to getCharge()
- In the Customer class, we need to update the reference to amountFor(each) to each.getCharge()
Here's the method in its new home:

public class Rental {
…
    public double getCharge() {

        double result = 0;

        switch (getMovie().getPriceCode()) {
            case Movie.REGULAR:
                result += 2;
                if (getDaysRented() > 2) {
                    result += (getDaysRented() - 2) * 1.5;
                }
                break;
            case Movie.NEW_RELEASE:
                result += getDaysRented() * 3;
                break;
            case Movie.CHILDRENS:
                result += 1.5;
                if (getDaysRented() > 3) {
                    result += (getDaysRented() - 3) * 1.5;
                }
                break;
        }

        return result;
    }
…
}

New Class Diagram

Step3 Classdiagram

No major changes; however Customer is now a smaller class and Rental has stopped being “just a data holder” and can now actually do something!

Definitely making progress!

Step 4: Replace Temp with Query

In the statement() method, thisAmount is now redundant
- It is initialized with the call to each.getCharge() and is not changed afterward
Lets get rid of it with the Replace Temp with Query refactoring discussed in the previous lecture
statement() now looks like this:

public String statement() {

    double      totalAmount          = 0;
    int         frequentRenterPoints = 0;
    Enumeration rentals              = _rentals.elements();
    String      result               = "Rental Record for " + getName() + "\n";

    while (rentals.hasMoreElements()) {

        Rental each       = (Rental) rentals.nextElement();

        // add frequent renter points
        frequentRenterPoints++;

        // add bonus for a two day new release rental
        if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
            (each.getDaysRented() > 1)) {
                frequentRenterPoints++;
        }

        // show figures for this rental
        result += "\t" + each.getMovie().getTitle() +
                  "\t" + String.valueOf(each.getCharge()) + "\n";
        totalAmount += each.getCharge();
    }

    // add footer lines
    result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
    result += "You earned " + String.valueOf(frequentRenterPoints) +
              " frequent renter points";
    return result;
}

Of course, now the getCharge() method is being called twice per Rental, where previously it was only called once per Rental
- We will not worry about this potential problem, however, until a profiler tells us that its an actual problem!

Step 5: Frequent Renter Points

Now lets do similar steps for the logic that calculates frequent renter points
In particular, this code (currently contained in statement())

// add frequent renter points
frequentRenterPoints++;

// add bonus for a two day new release rental
if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
    (each.getDaysRented() > 1)) {
        frequentRenterPoints++;
}

Step 5a: Extract Method
- Put the code for calculating frequent renter points into its own method
- As with last time, the variable each becomes a parameter of this method
- Modify code to just calculate the correct amount of points
  - We can no longer modify the getFrequentRenterPoints variable directly
- The new method looks like this:

public int getFrequentRenterPoints(Rental each) {
    if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
        (each.getDaysRented() > 1)) {
        return 2;
    } else {
        return 1;
    }
}

Step 5a continued
- Change statement() to call new method and update variable getFrequentRenterPoints like this

frequentRenterPoints += getFrequentRenterPoints(each);

Step 5b: Move Method
- Again, we are only using information from Rental in our newly extracted method
- So, lets move it over! Here is the new method in its new home:

public int getFrequentRenterPoints() {
    if ((getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
        (getDaysRented() > 1)) {
        return 2;
    } else {
        return 1;
    }
}

statement() now looks like this:

public String statement() {

    double      totalAmount          = 0;
    int         frequentRenterPoints = 0;
    Enumeration rentals              = _rentals.elements();
    String      result               = "Rental Record for " + getName() + "\n";

    while (rentals.hasMoreElements()) {

        Rental each       = (Rental) rentals.nextElement();

        frequentRenterPoints += each.getFrequentRenterPoints();

        // show figures for this rental
        result += "\t" + each.getMovie().getTitle() +
                  "\t" + String.valueOf(each.getCharge()) + "\n";
        totalAmount += each.getCharge();
    }

    // add footer lines
    result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
    result += "You earned " + String.valueOf(frequentRenterPoints) +
              " frequent renter points";
    return result;
}

New Class Diagram

Step5b Classdiagram

Customer continues to get smaller (but with better structure)

Rental continues to get larger (but more useful)

Step 6: Remove Temp Variables

statement() still has temp variables
- totalAmount and frequentRentalPoints
Both of these values are going to be needed by statement() and htmlStatement()
- Lets replace them with query methods
  - little more difficult because they were calculated within a loop; we have to move the loop to the query methods

Step 6a: Replace totalAmount

First, create a new query method called getTotalCharge()

private double getTotalCharge() {
    double result = 0;
    Enumeration rentals = _rentals.elements();
    while (rentals.hasMoreElements()) {
        Rental each = (Rental) rentals.nextElement();

        result += each.getCharge();
    }
    return result;
}

Second, remove all code in statement() that calculated the value of totalAmount and replace any references to the variable with calls to the new query method:

public String statement() {

int         frequentRenterPoints = 0;
Enumeration rentals              = _rentals.elements();
String      result               = "Rental Record for " + getName() + "\n";

while (rentals.hasMoreElements()) {

    Rental each       = (Rental) rentals.nextElement();

    frequentRenterPoints += each.getFrequentRenterPoints();

    // show figures for this rental
    result += "\t" + each.getMovie().getTitle() +
              "\t" + String.valueOf(each.getCharge()) + "\n";
}

// add footer lines
result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n";
result += "You earned " + String.valueOf(frequentRenterPoints) +
          " frequent renter points";
return result;
}

Step 6b: Replace frequentRentalPoints

Extract Method:

private int getTotalFrequentRenterPoints() {
    int result = 0;
    Enumeration rentals = _rentals.elements();
    while (rentals.hasMoreElements()) {
        Rental each = (Rental) rentals.nextElement();

        result += each.getFrequentRenterPoints();
    }
    return result;
}

Update statement():

public String statement() {

    Enumeration rentals              = _rentals.elements();
    String      result               = "Rental Record for " + getName() + "\n";

    while (rentals.hasMoreElements()) {

        Rental each       = (Rental) rentals.nextElement();

        // show figures for this rental
        result += "\t" + each.getMovie().getTitle() +
                  "\t" + String.valueOf(each.getCharge()) + "\n";
    }

    // add footer lines
    result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n";
    result += "You earned " + String.valueOf(getTotalFrequentRenterPoints()) +
              " frequent renter points";
    return result;
}

New Class Diagram

Step6b Classdiagram

Customer's structure continues to evolve; it now has two methods that can be reused across statement() and htmlstatement()

Note: statement()'s algorithm has now evolved. It used to perform the loop only once, now it performs it three times! Is this a performance problem? Maybe, we can't say without checking the program with a profiler

Step 7: add htmlstatement()

We can now add the htmlstatement() method to generate a statement in HTML format

public String htmlStatement() {

    Enumeration rentals              = _rentals.elements();
    String      result               = "<html><head><title>Statement</title></head><body>\n";

    result += "<p><h1>Rental Record for <em>" + getName() + "</em></h1></p>\n";

    while (rentals.hasMoreElements()) {

        Rental each       = (Rental) rentals.nextElement();

        // show figures for this rental
        result += "\t" + each.getMovie().getTitle() +
                  "\t" + String.valueOf(each.getCharge()) + "<br />\n";
    }

    // add footer lines
    result += "<p>Amount owed is <em>" + String.valueOf(getTotalCharge()) + "</em></p>\n";
    result += "<p>You earned <em>" + String.valueOf(getTotalFrequentRenterPoints()) +
              "</em> frequent renter points</P>\n";

    result += "</body></html>";
    return result;
}

Note: the similarity in the structure of statement() and htmlstatement(). This is a type of duplication that can be addressed with a refactoring called “Form Template Method”, but I will not cover that refactoring in detail today. (Outline of solution on next slide.)
We now need to add a test case to test this new functionality. Look in the step7 directory of the code for this tutorial (available on the class website)

Outline of Form Template Method Solution

FormTemplateMethod

Steps of Solution: Create new class hierarchy containing a single version of the duplicated method. Extract the portions that vary into new smaller methods. These methods are declared abstract on the superclass. The original method invokes these abstract methods at the appropriate place. These abstract methods are implemented in the subclasses.

New Requirements

It is now anticipated that the store is going to have more than the three initial types of movies
As a result of these new classifications, renter points and charges will vary with each new movie type
As a result, we should probably move the getCharge() and getFrequentRenterPoints() methods to the Movie class

Step 8: Move Methods

Move getCharge() and getFrequentRenterPoints() to the Movie class. Note: these routines need access to getDaysRented(); pass as parameter

Here are the methods in their new home:

public class Movie {
…
    double getCharge(int daysRented) {

        double result = 0;

        switch (getPriceCode()) {
            case Movie.REGULAR:
                result += 2;
                if (daysRented > 2) {
                    result += (daysRented - 2) * 1.5;
                }
                break;
            case Movie.NEW_RELEASE:
                result += daysRented * 3;
                break;
            case Movie.CHILDRENS:
                result += 1.5;
                if (daysRented > 3) {
                    result += (daysRented - 3) * 1.5;
                }
                break;
        }

        return result;
    }

    public int getFrequentRenterPoints(int daysRented) {
        if ((getPriceCode() == Movie.NEW_RELEASE) && (daysRented > 1)) {
            return 2;
        } else {
            return 1;
        }
    }
…

Here is what these methods look like in Rental after the move:

public double getCharge() {
    return getMovie().getCharge(getDaysRented());
}

public int getFrequentRenterPoints() {
    return getMovie().getFrequentRenterPoints(getDaysRented());
}

Since both of these methods are used by Customer, they need to remain in Rental and now they simply delegate to the methods in Movie

New Class Diagram

Step8b Classdiagram

Movie has new methods, finally making the transition from data holder to object; these methods will allow us to handle new types of movie easily

How to handle new Movies?

Movies Take1

But movies can change type! A children’s movie when it is first released is a “new release”; later it becomes a children’s movie. So this approach won’t work!

State pattern to the rescue!

Movies Take2

A movie has a particular state: its charge (and its renter points) depend on that state; so we can use the state pattern to handle new types of movies (for now, at least)

Note: In this formulation, the differences between the State pattern and the Strategy pattern we saw in a previous lecture are negligible

Step 9: Replace Type Code with State/Strategy

We need to get rid of our Type Code (e.g. Movie.CHILDRENS) and replace it with a Price object

We first modify Movie to get rid of its _priceCode field and replace it with a _price object
- this involves changing the constructor to make use of the setPriceCode() method; before it was setting _priceCode directly
- we also have to change getPriceCode and setPriceCode to access the Price object

Movie now looks like this:

public class Movie {

    public static final int CHILDRENS   = 2;
    public static final int REGULAR     = 0;
    public static final int NEW_RELEASE = 1;

    private String   _title;
    private Price    _price;

    public Movie(String title, int priceCode) {
        _title = title;
        setPriceCode(priceCode);
    }

    public int getPriceCode() {
        return _price.getPriceCode();
    }

    public void setPriceCode(int arg) {
        switch (arg) {
            case REGULAR:
                _price = new RegularPrice();
                break;
            case CHILDRENS:
                _price = new ChildrensPrice();
                break;
            case NEW_RELEASE:
                _price = new NewReleasePrice();
                break;
            default:
                throw new IllegalArgumentException("Incorrect Price Code");
        }
    }
…
}

We of course need to create Price and its subclasses:

public abstract class Price {
    abstract int getPriceCode();
}

public class RegularPrice extends Price {
    int getPriceCode() {
        return Movie.REGULAR;
    }
}

public class NewReleasePrice extends Price {
    int getPriceCode() {
        return Movie.NEW_RELEASE;
    }
}

public class ChildrensPrice extends Price {
    int getPriceCode() {
        return Movie.CHILDRENS;
    }
}

Step 10: Move Method

Now we need to move getCharge() to the Price class

public abstract class Price {
    abstract int getPriceCode();

    public double getCharge(int daysRented) {

        double result = 0;

        switch (getPriceCode()) {
            case Movie.REGULAR:
                result += 2;
                if (daysRented > 2) {
                    result += (daysRented - 2) * 1.5;
                }
                break;
            case Movie.NEW_RELEASE:
                result += daysRented * 3;
                break;
            case Movie.CHILDRENS:
                result += 1.5;
                if (daysRented > 3) {
                    result += (daysRented - 3) * 1.5;
                }
                break;
        }

        return result;
    }

}

Its a very simple move, we just need to change Movie to delegate its getCharge() method to Price
```
double getCharge(int daysRented) {
    return _price.getCharge(daysRented);
}
```

Step 11: Replace Conditional with Polymorphism

Now, we move each branch of the switch statement into the appropriate subclass
- I do this in one move; Fowler actually recommends moving one branch at a time, and introducing a bug, so you can be sure that the subclass’s method is the one being invoked!
After you have done the move, change Price’s getCharge to an abstract method

Price and its subclasses now look like this:

public abstract class Price {

    public abstract int getPriceCode();

    public abstract double getCharge(int daysRented);

}

public class NewReleasePrice extends Price {
    int getPriceCode() {
        return Movie.NEW_RELEASE;
    }

    double getCharge(int daysRented) {
        return daysRented * 3;
    }

}

public class RegularPrice extends Price {
    int getPriceCode() {
        return Movie.REGULAR;
    }

    double getCharge(int daysRented) {
        double result = 2;
        if (daysRented > 2) {
            result += (daysRented - 2) * 1.5;
        }
        return result;
    }

}

public class ChildrensPrice extends Price {
    int getPriceCode() {
        return Movie.CHILDRENS;
    }

    double getCharge(int daysRented) {
        double result = 1.5;
        if (daysRented > 3) {
            result += (daysRented - 3) * 1.5;
        }
        return result;
    }

}

Step 12: Handle Renter Points

Now we repeat step 10 and 11, this time applying them to frequent renter points
I combine both steps into one
- we move the method over to price, and use polymorphism to handle the logic
- note: this time we leave a default implementation in Price and have newRelease override that implementation, since it is the only class that returns a different value

Movie's getFrequentRenterPoints() method now looks like this:

public int getFrequentRenterPoints(int daysRented) {
    return _price.getFrequentRenterPoints(daysRented);
}

Price has the following method:

public int getFrequentRenterPoints(int daysRented) {
    return 1;
}

And NewReleasePrice now has the following method:

public int getFrequentRenterPoints(int daysRented) {
    return (daysRented > 1) ? 2 : 1;
}

Run the test; everything still works!

We’re done!

We’ve added new functionality, changed “data holders” to “objects” and made it very easy to add new types of movies with special charges and frequent rental points
The final version of the code is in the after directory; compile it and run the test: test passed!

Final class diagram

After

Refactoring

Refactoring is a simple, easy-to-adopt technique that helps improve the structure of a software system without changing its functionality
The Refactoring book contains 72 refactoring patterns that you can learn to help improve your design skills
In order to make refactoring safe, it should be performed in the presence of test cases
Agile techniques encourage writing test cases before writing code, as we will see in our next lecture on Test-Driven Design

Coming Up Next

Lecture 27: Test Driven Design
Lecture 28: Getting Real, Part 1

Foundations of Software Engineering

Lecture 26: Refactoring, Part 2

Credit where Credit is Due

Last Lecture

Goals of this Lecture

Tutorial: Background Information

Does this system need refactoring?

Initial Class Diagram

Initial Code: Movie

Initial Code: Rental

Initial Code: Customer

Step 1: Refactor statement()

But WAIT!!!

Our Test Case

Our Test Statement

Test Run on Original Program

Back to Step 1

Step 1 in Detail: Original Code

Step 1 in Detail: Extract Method

Step 1 in Detail: Update statement to call new method

Possible Pitfall

Step 2: Rename variables

Step 3: Move Method

New Class Diagram

Step 4: Replace Temp with Query

Step 5: Frequent Renter Points

New Class Diagram

Step 6: Remove Temp Variables

New Class Diagram

Step 7: add htmlstatement()

Outline of Form Template Method Solution

New Requirements

Step 8: Move Methods

New Class Diagram

How to handle new Movies?

State pattern to the rescue!

Step 9: Replace Type Code with State/Strategy

Step 10: Move Method

Step 11: Replace Conditional with Polymorphism

Step 12: Handle Renter Points

We’re done!

Final class diagram

Refactoring

Coming Up Next