What does refactoring actually look like? Let’s continue an example based on a real iOS view controller. I gave a talk for CocoaHeads NL called “Real Refactoring” where I demonstrated refactoring a view controller from MVC (model-view-controller) to MVVM (model-view-view-model). At least I started to, but for the sake of time, I cut the demo short.
A YouTube commenter said, “Too bad the talk ends just before the more interesting part where there’s actual logic in the item method. Was curious how you would pull these apart.” So in this video, let’s continue the demo from where it left off.
There will be more videos playing around with this code, from the Null Object Pattern to Model-View-Presenter. Subscribe to receive email notifications so you don’t miss the next video.
Brief Context: Sale Items at eBay
If you haven’t seen the previous video I really encourage you to go back and check it out. It’ll give you more context for what’s going on.
Here’s a quick refresher: We’re working at eBay, which has items listed on its site. An item can have a title, an image, and a price… a bunch of other stuff, of course. Some items are on special sale. A sale item has those same things—the title, the image, and the price. But it also has the original price with a strikethrough line, to show you what a discount you’re getting.
Let’s continue from the hiding and showing of this strikethrough price label.
AppCode, not Xcode?!
You may be startled to see me using an IDE you don’t recognize. I use AppCode, a more powerful IDE for the kind of refactoring I do daily.
If you’d like to try this refactoring yourself, get the code from GitHub. If you want to try it in AppCode, then download the AppCode EAP (Early Access Program) if it’s available. The EAP is a free prerelease that expires when they turn it a proper release.
Going from Xcode to AppCode, the main thing you need to know is how to run tests. AppCode doesn’t use Xcode’s schemes. Instead, you set up an XCTest configuration. From there, “Build” builds the the test code and the production code it depends on. “Run” does a build, then runs the tests. For more help getting started with AppCode, see the AppCode Quick Start Guide.
For this particular project, make sure to select an iPad simulator as the destination. Run tests. You’ll see it pass 10 unit tests which fully test this part of our view controller. The unit tests act as a safety net, allowing us to refactor. They are a critical piece of real refactoring.
Moving the Attributed Text
Uncovering a Side Effect
In part one of the demo, I took advantage of a recurring pattern. On one side of the if-statement, the code set a view property. On the else-side, it cleared that same property, setting it to nil.
As the view model evolved, this behavior gradually moved over there. Both the if and the else clause came to set the property from the same computed property in the view model. Then I could fold them together into a single line.
But this isn’t the case for the strikethrough price label. On the else side, the attributedText is set to nil. But on the if side, it doesn’t appear to set this property. Its appearance is misleading, but there is a hint in the method name: setStrikethroughText. When we go to the implementation, we can see that it sets the attributedText property.
This imbalance is curious and hinders the course of the refactoring. The method sets a property as a side effect, instead of making the side effect clear. I find this to be a warning against making extensions like this on objects you don’t own—in this case, UILabel.
extension UILabel {
func setStrikethroughText(_ text: String) {
// ...
}
}So the first step in refactoring this is to turn it from a hidden side effect into a plain old setting of a property. We do this by creating a standalone function to create the attributed text we want, then assigning that to the property.
func strikethroughText(_ text: String) -> NSAttributedString {
// ...
}Quickly Creating the New Function
To create the function itself, I use an AppCode technique. At the call site, I call the new, non-existent function. I pretend that it exists, and make sure that it looks clear at the point of use. Then with a click, I ask AppCode to create an outline of the new function. It figures out the signatures and generates a skeleton for us to fill in.
We create the body of the new function by copying and pasting the old code, then fixing it up to fit into its new home. Another AppCode technique I use is to increase or decrease the currently selected scope. This makes it easy to select the right portion of code.
With the new call in place and the new implementation in place, we can shift away from the old code. When I’m not entirely certain if something will work, I leave the old code in place, commented out. That way if something goes wrong, I can find what to restore by looking for it. And if all is well, I delete the commented-out code.
Moving the Free Function into a Namespace
The names of freestanding functions exist in the module’s namespace. Rather than have them live out in the open, I prefer to put free functions inside namespaces when I work on large projects.
Swift namespaces are implicit and normally invisible. But it’s easy to use an empty enumeration as an explicit namespace. Move the free function into the enum, and declare it to be static. Then the caller can reference the function by its full name.
For a single function or a small project this doesn’t matter, as the name of the new enum itself goes in the module’s namespace. But this can be a helpful way to organize free functions into groups as you get more of them.
enum AttributedText {
static func strikethroughText(_ text: String) -> NSAttributedString {
// ...
}
}Moving Code Around in Small, Verified Steps
So far we’ve taken dissimilar code and made it more similar. Now both the if-clause and the else-clause make assignments to the label’s attributedText. The clearing-up has worked, so now we can move this behavior into the view model. I create a new computed property in the view model. The initial implementation does nothing, but it does compile.
Then we use the same trick I showed in part one:
- Copy the entire if-else statement.
- Paste it into its new home.
- Trim it down by removing anything unrelated to the attributedText.
- Fix it up by changing the assignments to return statements.
- Where there are missing returns, add them in with an appropriate do-nothing value.
Then we shift the assignments to use this new computed property in the view model. One by one, I do the replacement and run tests. Eventually, both lines look the same, in both the if clause and the else clause:
if let item = item {
strikethroughPriceLabel.attributedText = viewModel.strikethroughPrice
// more stuff...
} else {
strikethroughPriceLabel.attributedText = viewModel.strikethroughPrice
// possibly other stuff...
}Now we can lift it out of the if statement altogether. This changes the order of the statements, moving the assignment ahead of the conditional. As long as this isn’t a problem, we can lift both statements out into a single statement outside the if. Effectively, this is the same as the Slide Statements refactoring from the Refactoring book.
With a good set of unit tests, we can do these refactoring steps with little thought or analysis.
With a good set of unit tests, we can do these refactoring steps with little thought or analysis. All we’re doing is moving code around. As long as the test pass, we’re good.
Finally, we can clear up the implementation in the view model. A guard clause seems like a helpful way to make the code more expressive.
In the end, the code still isn’t as clear as I’d like it to be. But sometimes it’s best not to “clean all the things” at once. Continue with other refactoring. As common patterns emerge, we can extract those later as we discover them. This after-the-fact discovery happens often.
Moving the “isHidden”
Making similar changes, we can move the label’s isHidden value. I encourage you to practice these steps on your own computer.
- Determine what return type we need for this value. In this case, we want a Bool.
- Define the skeleton of a computed property in the view model that returns this type. Come up with a good name. Give it a bare-bones implementation that builds.
- Copy the code and paste it into its new home. We’re down to the last property, so there’s no remaining code that sets other properties. Change it from making assignments to returning values. Add extra return statements where needed. Get it to build.
- Change one call site to use the new computed property. Confirm by running tests.
- Change the other call site and run tests.
- Now that both calls are identical, lift them out of the conditional and run tests.
Then it’s a journey of clearing up the implementation. We can shape the code to be similar to the other property. Where we fine similar “shapes” in the code, we can extract helper functions.
Here, both Xcode and AppCode support “Extract Method.” The results are slightly different, though.
- In Xcode, the extracted method comes above the call site and is marked fileprivate.
- AppCode instead displays a dialog to let you decide the accessibility, the name, the parameter names, and parameter order. It then creates the new code below the call site.
Having the calling code above the helper code reads better to me because it’s top-down.
More to Do, For Me & You
As it stands, the view model keeps checking whether the item is nil. This is crying out to use the Null Object Pattern to eliminate those conditionals. I do that refactoring in a separate video.
While I was at eBay writing the code this is based on, I didn’t go to model-view-view-model (MVVM). Instead, I went to model-view-presenter (MVP). I plan to show that in yet another video.
As you can see there’s a lot to mine in this example. I encourage you to get the code and try refactoring it yourself. You may end up with different results, which is totally fine. But make sure to move in small, verified steps. I want you to experience the power of having a fully-tested view controller.
See the next video where I refactor this further using the Null Object Pattern.
