Refactoring: Improving the Design of Existing Code (Addison-Wesley Signature Series (Fowler))

by Martin Fowler

Data structures are often unpacked unnecessarily when passed between functions; I prefer to keep them together with Preserve Whole Object (319). Decisions on what should be passed as a parameter, and what can be resolved by the called function, are ones I often need to revisit with Replace Parameter with Query (324) and Replace Query with Parameter (327).

A class is a common form of module. I prefer my objects to be as immutable as possible, so I use Remove Setting Method (331) whenever I can. Often, when a caller asks for a new object, I need more flexibility than a simple constructor gives, which I can get by using Replace Constructor with Factory Function (334).

It is a good idea to clearly signal the difference between functions with side effects and those without. A good rule to follow is that any function that returns a value should not have observable side effects—the command-query separation [mf-cqs]. Some programmers treat this as an absolute rule. I’m not 100 percent pure on this (as on anything), but I try to follow it most of the time, and it has served me well.

A flag argument is a function argument that the caller uses to indicate which logic the called function should execute.

My first route into an API is usually the list of available functions, and flag arguments hide the differences in the function calls that are available. Once I select a function, I have to figure out what values are available for the flag arguments. Boolean flags are even worse since they don’t convey their meaning to the reader—in a function call, I can’t figure out what true means. It’s clearer to provide an explicit function for the task I want to do.

It’s easier to reason about a function that will always give the same result when called with same parameter values—this is called referential transparency. If a function accesses some element in its scope that isn’t referentially transparent, then the containing function also lacks referential transparency. I can fix that by moving that element to a parameter. Although such a move will shift responsibility to the caller, there is often a lot to be gained by creating clear modules with referential transparency. A common pattern is to have modules consisting of pure functions which are wrapped by logic that handles the I/O and other variable elements of a program. I can use Replace Query with Parameter to purify parts of a program, making those parts easier to test and reason about.

A problem with JavaScript’s class model is that it’s impossible to enforce an immutable class—there’s always a way to get at an object’s data. But writing a class to signal and encourage immutability is often good enough. Creating classes that have this characteristic is often a sound strategy and Replace Query with Parameter is a handy tool for doing this.

Providing a setting method indicates that a field may be changed. If I don’t want that field to change once the object is created, I don’t provide a setting method (and make the field immutable). That way, the field is set only in the constructor, my intention to have it not change is clear, and I usually remove the very possibility that the field will change. There’s a couple of common cases where this comes up. One is where people always use accessor methods to manipulate a field, even within constructors. This leads to the only call to a setting method being from the constructor. I prefer to remove the setting method to make it clear that updates make no sense after construction.

Many object-oriented languages have a special constructor function that’s called to initialize an object. Clients typically call this constructor when they want to create a new object. But these constructors often come with awkward limitations that aren’t there for more general functions. A Java constructor must return an instance of the class it was called with, which means I can’t replace it with a subclass or proxy depending on the environment or parameters. Constructor naming is fixed, which makes it impossible for me to use a name that is clearer than the default. Constructors often require a special operator to invoke (“new” in many languages) which makes them difficult to use in contexts that expect normal functions.

But I don’t like using the type code here—it’s generally a bad smell to pass a code as a literal string. So I prefer to create a new factory function that embeds the kind of employee I want into its name.

Functions—either freestanding or attached to objects as methods—are one of the fundamental building blocks of programming. But there are times when it’s useful to encapsulate a function into its own object, which I refer to as a “command object” or simply a command. Such an object is mostly built around a single method, whose request and execution is the purpose of the object.

A command offers a greater flexibility for the control and expression of a function than the plain function mechanism. Commands can have complimentary operations, such as undo. I can provide methods to build up their parameters to support a richer lifecycle. I can build in customizations using inheritance and hooks. If I’m working in a language with objects but without first-class functions, I can provide much of that capability by using commands instead. Similarly, I can use methods and fields to help break down a complex function, even in a language that lacks nested functions, and I can call those methods directly while testing and debugging.

Like many words in software development, “command” is rather overloaded. In the context I’m using it here, it is an object that encapsulates a request, following the command pattern in Design Patterns [gof]. When I use “command” in this sense, I use “command object” to set the context, and “command” afterwards. The word “command” is also used in the command-query separation principle [mf-cqs], where a command is an object method that changes observable state. I’ve always tried to avoid using command in that sense, preferring “modifier” or “mutator.”

The JavaScript language has many faults, but one of its great decisions was to make functions first-class entities. I thus don’t have to go through all the hoops of creating commands for common tasks that I need to do in languages without this facility. But there are still times when a command is the right tool for the job.

Indeed, when doing this refactoring in JavaScript, using nested functions would be a reasonable alternative to using a command. I’d still use a command for this, partly because I’m more familiar with commands and partly because with a command I can write tests and debugging calls against the subfunctions.

Command objects provide a powerful mechanism for handling complex computations. They can easily be broken down into separate methods sharing common state through the fields; they can be invoked via different methods for different effects; they can have their data built up in stages.

Eliminating duplicate code is important. Two duplicate methods may work fine as they are, but they are nothing but a breeding ground for bugs in the future. Whenever there is duplication, there is risk that an alteration to one copy will not be made to the other. Usually, it is difficult to find the duplicates. The easiest case of using Pull Up Method is when the methods have the same body, implying there’s been a copy and paste. Of course it’s not always as obvious as that. I could just do the refactoring and see if the tests croak—but that puts a lot of reliance on my tests. I usually find it valuable to look for the differences—often, they show up behavior that I forgot to test for. Often, Pull Up Method comes after other steps. I see two methods in different classes that can be parameterized in such a way that they end up as essentially the same method.

If subclasses are developed independently, or combined through refactoring, I often find that they duplicate features. In particular, certain fields can be duplicates. Such fields sometimes have similar names—but not always. The only way I can tell what is going on is by looking at the fields and examining how they are used. If they are being used in a similar way, I can pull them up into the superclass. By doing this, I reduce duplication in two ways. I remove the duplicate data declaration and I can then move behavior that uses the field from the subclasses to the superclass.

If a method is only relevant to one subclass (or a small proportion of subclasses), removing it from the superclass and putting it only on the subclass(es) makes that clearer. I can only do this refactoring if the caller knows it’s working with a particular subclass—otherwise, I should use Replace Conditional with Polymorphism (272) with some placebo behavior on the superclass.

Software systems often need to represent different kinds of a similar thing. I may classify employees by their job type (engineer, manager, salesman), or orders by their priority (rush, regular). My first tool for handling this is some kind of type code field—depending on the language, that might be an enum, symbol, string, or number. Often, this type code will come from an external service that provides me with the data I’m working on. Most of the time, such a type code is all I need. But there are a couple of situations where I could do with something more, and that something more are subclasses. There are two things that are particularly enticing about subclasses.

Subclasses are useful. They support variations in data structure and polymorphic behavior. They are a good way to program by difference. But as a software system evolves, subclasses can lose their value as the variations they support are moved to other places or removed altogether. Sometimes, subclasses are added in anticipation of features that never end up being built, or end up being built in a way that doesn’t need the subclasses. A subclass that does too little incurs a cost in understanding that is no longer worthwhile. When that time comes, it’s best to remove the subclass, replacing it with a field on its superclass.

When I’m refactoring a class hierarchy, I’m often pulling and pushing features around. As the hierarchy evolves, I sometimes find that a class and its parent are no longer different enough to be worth keeping separate. At this point, I’ll merge them together.

There is a popular principle: “Favor object composition over class inheritance” (where composition is effectively the same as delegation). Many people take this to mean “inheritance considered harmful” and claim that we should never use inheritance. I use inheritance frequently, partly because I always know I can use Replace Subclass with Delegate should I need to change it later. Inheritance is a valuable mechanism that does the job most of the time without problems. So I reach for it first, and move onto delegation when it starts to rub badly. This usage is actually consistent with the principle—which comes from the Gang of Four book [gof] that explains how inheritance and composition work together.

In JavaScript, accessing a property on an object where it isn’t defined returns undefined, so I do that here. (Although my every instinct is to have it raise an error, which would be the case in other object-oriented dynamic languages I’m used to.) Once I’ve moved all the behavior out of the subclass, I can change the factory method to return the superclass—and, once I’ve run tests to ensure all is well, delete the subclass.

In object-oriented programs, inheritance is a powerful and easily available way to reuse existing functionality. I inherit from some existing class, then override and add additional features. But subclassing can be done in a way that leads to confusion and complication.