A field's type (e.g., a String or int) fails to protect it from unsafe assignments and invalid equality comparisons.
Constrain the assignments and equality comparisons by making the type of the field a class.
Motivation A primary motivation for refactoring from a type code to a class is to make code type-safe. One way to do that is to constrain the possible values that may be assigned to or equated with a field or variable. Consider the following type-unsafe code: public void testDefaultsToPermissionRequested() { SystemPermission permission = new SystemPermission(); assertEquals(permission.REQUESTED, permission.state()); assertEquals("REQUESTED", permission.state()); }
This code creates a SystemPermission object. The constructor for this object sets its state field equal to the SystemPermission.REQUESTED state: public SystemPermission() { state = REQUESTED; }
Other methods within SystemPermission assign state to system permission states such as GRANTED and DENIED. Given that each of these state types was defined using String constants (like public final static String REQUESTED = "REQUESTED") and state was defined as type String, the two assert statements above both pass because the state field, which is accessed via the call to permission.state(), is considered equal to both SystemPermission.REQUESTED and the String, "REQUESTED." What's the problem with this? The problem is that using a String value in this context is error prone. For example, what if an assert statement were accidently written like this: assertEquals("REQEUSTED", permission.state());
Would that assert pass? No, for the String, "REQEUSTED", has been accidently misspelled! Using a String as the type for SystemPermission's state field also leaves it exposed to errors like this one: public class SystemPermission... public void setState(String newState){ state = newState; } permission.setState("REQESTED"); // another misspelling of "REQUESTED"
Here, the misspelling of "REQESTED" will not cause a compiler error but will allow a SystemPermission instance to enter an invalid state. Once it enters that invalid state, SystemPermission's state transition logic won't allow it to ever leave that invalid state. Using a class instead of a String to represent SystemPermission's state field will reduce such errors because it ensures that all assignments and comparisons are performed using a family of type-safe constants. These constants are considered to be type-safe because their type, which is a class, protects them from being impersonated by other constants. For example, in the following code, the type-safe constant, REQUESTED, lives in one class and can't be mistaken for any other value: public class PermissionState { public final static PermissionState REQUESTED = new PermissionState();
Clients that want to perform assignment or equality statements using REQUESTED can only obtain a reference to it by calling PermissionState.REQUESTED. Using a class to obtain type safety for a family of constants was described by Joshua Bloch as the Type-Safe Enum pattern [Bloch]. Joshua does an excellent job of explaining this pattern and how to deal with the serialization/deserialization issues associated with it. Languages that provide native support for what are commonly called "enums" might seem to render this refactoring useless. That isn't the case, for after you perform this refactoring, you'll often extend your code to support more behavior, which isn't possible with enums. For example, the first step in the mechanics for Replace State-Altering Conditionals with State (166) builds upon this refactoring but could not build upon a language-based enum. Benefits and Liabilities | + | Provides better protection from invalid assignments and comparisons. | | – | Requires more code than using unsafe type does. |
|
Mechanics In the steps below a type-unsafe constant is a constant defined using a primitive or non-class-based type, such as a String or int. - 1. Identify a type-unsafe field, a field declared as a primitive or non-class-based type that is assigned to or compared against a family of type-unsafe constants. Self-encapsulate the type-unsafe field by applying Self Encapsulate Field [F].
- 2. Create a new class, a concrete class that will soon replace the type used for the type-unsafe field. Name this class after the kinds of types it will store. For the moment, don't provide any constructor for the new class.
- 3. Choose a constant value that the type-unsafe field is assigned to and/or compared against and define a new version of this constant in your new class by creating a constant that is an instance of the new class. In Java, it's common to declare this constant to be public final static.
Repeat this step for all constant values assigned to or compared against the type-unsafe field.
You've now defined a family of constants in the new class. If it is important to prevent clients from adding members to that family of constants, declare a single private constructor for the new class or, if your language allows it, mark the new class as final.
- 4. In the class that declared the type-unsafe field, create a type-safe field, a field whose type is the new class. Create a setting method for it.
- 5. Wherever an assignment is made to the type-unsafe field, add a similar assignment statement to the type-safe field, using the appropriate constant in the new class.
- 6. Change the getter method for the type-unsafe field so that its return value is obtained from the type-safe field. This requires making the constants in the new class capable of returning the correct value.
- 7. In the class that declared the type-unsafe field, delete the type-unsafe field, the setter method for it, and all calls to the setting method.
- 8. Find all references to the type-unsafe constants and replace them with calls to the corresponding constant in the new class. As part of this step, change the getter method for the type-unsafe field so its return type is the new class and make all changes necessary to callers of the revised getter method.
Equality logic that once relied on primitives will now rely on comparing instances of the new class. Your language may provide a default way to do such equality logic. If not, write code to ensure that the equality logic works correctly with new class instances.
Delete the now unused type-unsafe constants.
Example This example, which was shown in the code sketch at the beginning of this refactoring and mentioned in the Motivation section, deals with handling permission requests to access software systems. We'll begin by looking at relevant parts of the class, SystemPermission: public class SystemPermission { private String state; private boolean granted; public final static String REQUESTED = "REQUESTED"; public final static String CLAIMED = "CLAIMED"; public final static String DENIED = "DENIED"; public final static String GRANTED = "GRANTED"; public SystemPermission() { state = REQUESTED; granted = false; } public void claimed() { if (state.equals(REQUESTED)) state = CLAIMED; } public void denied() { if (state.equals(CLAIMED)) state = DENIED; } public void granted() { if (!state.equals(CLAIMED)) return; state = GRANTED; granted = true; } public boolean isGranted() { return granted; } public String getState() { return state; } }
- 1. The type-unsafe field in SystemPermission is called state. It is assigned to and compared against a family of String constants also defined within SystemPermission. The goal is to make state type-safe by making its type be a class rather than a String.
I begin by self-encapsulating state:
public class SystemPermission... public SystemPermission() { setState(REQUESTED); granted = false; } public void claimed() { if ( getState().equals(REQUESTED)) setState(CLAIMED); } private void setState(String state) { this.state = state; } public String getState() { // note: this method already existed return state; } // etc.
This is a trivial change, and my compiler and tests are happy with it.
- 2. I create a new class and call it PermissionState because it will soon represent the state of a SystemPermission instance.
public class PermissionState { }
- 3. I choose one constant value that the type-unsafe field is assigned to or compared against and I create a constant representation for it in PermissionState. I do this by declaring a public final static in PermissionState that is an instance of PermissionState:
public final class PermissionState { public final static PermissionState REQUESTED = new PermissionState(); }
I repeat this step for each constant in SystemPermission, yielding the following code:
public class PermissionState { public final static PermissionState REQUESTED = new PermissionState(); public final static PermissionState CLAIMED = new PermissionState(); public final static PermissionState GRANTED = new PermissionState(); public final static PermissionState DENIED = new PermissionState(); }
The compiler accepts this new code.
Now I must decide whether I want to prevent clients from extending or instantiating PermissionState in order to ensure that the only instances of it are its own four constants. In this case, I don't need such a rigorous level of type safety, so I don't define a private constructor or use the final keyword for the new class.
- 4. Next, I create a type-safe field inside SystemPermission, using the PermissionState type. I also create a setter method for it:
public class SystemPermission... private String state; private PermissionState permission; private void setState(PermissionState permission) { this.permission = permission; }
- 5. Now I must find all assignment statements to the type-unsafe field, state, and make similar assignment statements to the type-safe field, permission:
public class SystemPermission... public SystemPermission() { setState(REQUESTED); setState(PermissionState.REQUESTED); granted = false; } public void claimed() { if (getState().equals(REQUESTED)) { setState(CLAIMED); setState(PermissionState.CLAIMED); } } public void denied() { if (getState().equals(CLAIMED)) { setState(DENIED); setState(PermissionState.DENIED); } } public void granted() { if (!getState().equals(CLAIMED)) return; setState(GRANTED); setState(PermissionState.GRANTED); granted = true; }
I confirm that the compiler is OK with these changes.
- 6. Next, I want to change the getter method for state to return a value obtained from the type-safe field, permission. Because the getter method for state returns a String, I'll have to make permission capable of returning a String as well. My first step is to modify PermissionState to support a toString() method that returns the name of each constant:
public class PermissionState { private final String name; private PermissionState(String name) { this.name = name; } public String toString() { return name; } public final static PermissionState REQUESTED = new PermissionState( "REQUESTED"); public final static PermissionState CLAIMED = new PermissionState( "CLAIMED"); public final static PermissionState GRANTED = new PermissionState( "GRANTED"); public final static PermissionState DENIED = new PermissionState( "DENIED"); }
I can now update the getter method for state:
public class SystemPermission... public String getState() { return state; return permission.toString(); }
The compiler and tests confirm that everything is still working.
- 7. I can now delete the type-unsafe field, state, SystemPermission calls to its private setter method, and the setter method itself:
public class SystemPermission... private String state; private PermissionState permission; private boolean granted; public SystemPermission() { setState(REQUESTED); setState(PermissionState.REQUESTED); granted = false; } public void claimed() { if (getState().equals(REQUESTED)) { setState(CLAIMED); setState(PermissionState.CLAIMED); } } public void denied() { if (getState().equals(CLAIMED)) { setState(DENIED); setState(PermissionState.DENIED); } } public void granted() { if (!getState().equals(CLAIMED)) return; setState(GRANTED); setState(PermissionState.GRANTED); granted = true; } private void setState(String state) { this.state = state; }
I test that SystemPermission still works as usual. It does.
- 8. Now I replace all code that references SystemPermission's type-unsafe constants with code that references PermissionState's contant values. For example, SystemPermission's claimed() method still references the "REQUESTED" type-unsafe constant:
public class SystemPermission... public void claimed() { if (getState().equals(REQUESTED)) // equality logic with type-unsafe constant setState(PermissionState.CLAIMED); }
I update this code as follows:
public class SystemPermission... public PermissionState getState() { return permission .toString(); } public void claimed() { if (getState().equals( PermissionState.REQUESTED)) { setState(PermissionState.CLAIMED); }
I make similar changes throughout SystemPermission. In addition, I update all callers on getState() so that they now work exclusively with PermissionState constants. For example, here's a test method that requires updating:
public class TestStates... public void testClaimedBy() { SystemPermission permission = new SystemPermission(); permission.claimed(); assertEquals(SystemPermission.CLAIMED, permission.getState()); }
I change this code as follows:
public class TestStates... public void testClaimedBy() { SystemPermission permission = new SystemPermission(); permission.claimed(); assertEquals( PermissionState.CLAIMED, permission.getState()); }
After making similar changes throughout the code, I compile and test to confirm that the new type-safe equality logic works correctly.
Finally, I can safely delete SystemPermission's type-unsafe constants because they are no longer being used:
public class SystemPermission... public final static String REQUESTED = "REQUESTED"; public final static String CLAIMED = "CLAIMED"; public final static String DENIED = "DENIED"; public final static String GRANTED = "GRANTED";
Now SystemPermission's assignments to its permission field and all equality comparions with its permission field are type-safe. |
