DRAFT - DRAFT - DRAFT

Refactoring to Model-View-Controller

Abstract: So you've heard great things about the Model-View-Controller (MVC) design pattern, and you'd like to use it, but it turns out you're stuck with a nasty, messy, well-intentioned but ultimately poorly designed servlet. Never fear! You can get there from here...

Part 1
Part 2

The Dream: MVC

I'll be honest. I didn't used to know exactly what MVC means. I knew that the way people describe MVC is "model-view separation," which means, "separate the code that handles your data (the 'data model') from the code that handles displaying that data (the 'view')," and I knew that's a good thing. I also knew that the term "controller" is short for "code that handles the interaction between model and view, and/or functions outside the scope of either."

However, I also knew that people used the term "MVC" as a catchall. I discovered that there are many gradations of model-view separation, and not all of them count as MVC. (You can read about these gradations in my recent article MVC And Beyond.)

MVC is what's known as a Design Pattern: a generalized solution to a recurring problem. Since MVC is an abstract concept, there are many many ways to implement it in code. In a J2EE world, there are many specific MVC designs, including:

In this article, we will discuss moving from a one-servlet solution to an MVC separation using POJO technology (POJO, coined by Martin Fowler et al., stands for Plain Old Java Object). [1] Once we have achieved a cleaner separation between code functions, it will then be possible (though not necessarily desirable) to move to one of the more formal frameworks. For instance, after extracting a View object that generates HTML, we can replace it with a JSP solution; or, after extracting a Model object, we can replace it with an EJB (or, more likely, retool it into an EJB client).

Note that many systems claim to be MVC, but do not actually separate the controller functions into a separate object. Often the view performs controller-like or model-like functions. For instance, a JSP often has validation code (arguably a model function) or sends redirects (a controller function).

The Reality: Tangled Code

In Source Code Listing 1, we see a simple servlet. Its responsibility is to display the photos in a given photo album. If there are more than 20 photos, it will split the view up into separate pages, and display "previous" and "next" links as appropriate.

Note that we have already gone part of the way towards MVC separation. Instead of making direct SQL calls [2] inside the servlet, we instead have an object model (the classes Album, Picture, and User) representing the major domain objects. The precise nature of the persistence mechanism is encapsulated inside these objects, and the servlet neither knows nor cares how the superclass' loadAlbumByName method retrieves the data.

However, there is still a long way to go. The logical flow of the code is somewhat difficult to follow, and there is a fair amount of duplicated code; this means that adding new features, or fixing bugs, will be more difficult than it need be.

Moreover, the servlet as written is very difficult to test. Since it has essentially one input (the servlet parameters) and one output (raw HTML), the only way we can write tests is to pass in parameters and then parse the HTML returned. Writing GUI tests is very difficult. If the servlet had a cleaner design, we could perform more rigorous unit testing on the behind-the-scenes functionality. [!! lame -- needs to be punchier]

Refactoring To Understand

One of the most important uses of Refactoring is "Refactoring To Understand" -- manipulating code on-the-fly as you read through it, to make it cleaner and clearer without a specific design goal in mind. The rationale for this sort of "improvisational design" can be somewhat hard to swallow. Why would you want to change code that works, just in order to understand it better? But it usually turns out that refactoring-to-understand not only prepares you; it also prepares the code. Code which has been refactored-to-understand is almost always more modular, and thus more amenable to change (for maintenance, adding features, or applying larger-scale redesigns).

Step 1: Extract Methods

Our servlet is not literally one big method; some aspects of the program flow are represented as individual methods. However, it is still very hard to follow, and the doGet method is still way too long. I encourage you to download AlbumServlet1.java, load it into your favorite refactoring IDE (IDEA, Eclipse, and Emacs are all good choices), and refactor the heck out of it.

(Since every good refactorer needs a set of unit tests, you may want to download the files AlbumServletTest.java and expected.html as well. Every time you refactor, you may compile and run the tests with javac AlbumServletTest.java; java AlbumServletTest.)

As you go through the code, remember that the most useful refactorings are (in this order):

Feel free to rename any methods or variables to suit your idea of what their purpose is. However, this article will focus on the second-most-important refactoring, Extract Method.

First, attempt to extract methods by functional grouping. For example, the code block

        String pageParameter = request.getParameter("page");
        int page;
        if (pageParameter == null || pageParameter.equals(""))
        {
            page = 0;
        }
        else
        {
            page = Integer.parseInt(pageParameter);
        }
may be extracted into a method named getPageNumber.

You may also find code blocks that are identical, or contain only slight variations. These blocks can be extracted into methods that take the variations as parameters. For example, the two error conditions look suspiciously similar. Take the following big ugly block of code:

        if (albumName == null || albumName.equals(""))
        {
            // print error
            out.println("<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01 Transitional//EN\">");
            out.println("<html>");
            out.println("<head>");
            out.println("<title>Error</title>");
            out.println("<body>");
            out.println("<h1>Error</h1>");
            out.println("<p class='error'>");
            out.print("Missing parameter: album");
            out.print("</p></body>");
            out.println("</html>");
First apply the Introduce Variable refactoring on the string "Missing parameter: album":
        if (albumName == null || albumName.equals(""))
        {
            // print error
            String message = "Missing parameter: album";
            out.println("<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01 Transitional//EN\">");
            out.println("<html>");
            out.println("<head>");
            out.println("<title>Error</title>");
            out.println("<body>");
            out.println("<h1>Error</h1>");
            out.println("<p class='error'>");
            out.print(message);
            out.print("</p></body>");
            out.println("</html>");
Next, apply the Extract Method refactoring on the print statements:
        if (albumName == null || albumName.equals(""))
        {
            // print error
            String message = "Missing parameter: album";
            printError(out, message);
            return;
        }
Finally, you may call your new printError message in place of the other set of identical print statements. Inline the variable, and then, for the coup de grace, remove the now-redundant "// print error" comment.
        if (albumName == null || albumName.equals(""))
        {
            printError(out, "Missing parameter: album");
            return;
        }

        Album album = loadAlbumByName(albumName);
        if (album == null)
        {
            printError(out, "Could not load album '" + albumName + "'");
            return;
        }
Much clearer, no?

Step 2: Extract View Class

After some time spent refactoring, you may end up with a class that looks something like AlbumServlet2.java.

The next step is to construct a View object. Don't worry too much about the details of its responsibilities. For now, we just want to move the bulk of the display code into this new object. Create a class called AlbumPage, and move to this new class any method containing a print statement. Then go back into the AlbumServlet, and where previously, your code called:

printPage(out, album, request); 
instead, make it call
new AlbumPage().printPage(out, album, request);

Note that we're not getting bogged down in too much design right now. We just want a quick and dirty first pass, sort of a rough draft of our eventual pristine object model. Don't worry; we will have plenty of time to tart it up later.

This will still leave a few wrinkles to iron out. In my code, there were two unresolved symbols: getPageNumber() and PICTURES_PER_PAGE. For now, let's move these members into the new View object as well. However, make a note: since neither of these values has to do directly with rendering HTML, there is a good chance they do not belong inside the View object. Instead, they will soon be moved into the Model.

Detour: Extract Error View Class

If you look closely, you will see that the servlet only calls two methods of our new AlbumPage class: printPage and printError. Thinking about it, shouldn't printError be in a completely separate class? It is displaying a completely different type of page, after all.

Once we start extracting the ErrorPage class, we notice that it shares a method with AlbumPage: printHead. We could copy-and-paste it into both classes. However, that leads to duplicated code. So the very next step is to apply the Extract Superclass refactoring, and then to move the printHead() method into it.

This is a perfect example of so-called emergent design. Two similar classes become siblings with a newly-created common parent, and the parent has "just enough API" -- only the members actually shared by the children make it into the parent class. (See AlbumServlet3.java.)

Compare this to a more traditional top-down Object-Oriented Design process, where you often end up with placeholder superclasses. These hollow types capture a conceptual similarity between two or more child objects, but contain no salient functionality of their own. If misused, this up-front style of object design can lead to the dreaded Forest of Empty Objects, where vainly wandering among the myriad fruitless trees in search of a single sapling can be an exercise in frustration.

(Later on, capturing the similarity between printError and printPage, you may want to give Page an abstract printPage method, and rename printError to printPage. This can wait for now, however. Make a note and move on.)

Step 3: Parameterize View Class

After I extract a class, I like to go through it and convert common parameters of public methods to instance variables wherever possible; I then add a constructor which takes these values as parameters. This reduces the length of message signatures, which makes them a bit easier to understand. It also helps to clarify the class' dependencies -- the constructor signature now documents which classes this class needs to refer to. After applying that refactoring to the servlet, we end up moving from

public void printPage(PrintWriter out, Album album, HttpServletRequest request)
to the following:
    private Album album;
    private HttpServletRequest request;

    public AlbumPage(Album album, HttpServletRequest request)
    {
        this.album = album;
        this.request = request;
    }

    public void printPage(PrintWriter out)
...

Note that we could have gone further and extracted the PrintWriter as well. It is purely for aesthetic reasons that I chose to leave it as a parameter, since it feels correct for a print method to take a PrintWriter. However, you could go either way.

[Is there a name for this refactoring? I looked in refactoring.com/catalog but couldn't find anything like "Replace Parameter With Field" or "Move Parameter To Constructor"...]

On to Part 2


Alex Chaffee
Last modified: Wed Oct 23 12:58:26 PDT 2002