Refactoring, Part 2

I read most of the way through the rest of Fowler's refactoring: almost all the way through the catalog. I don't have much of a response to the refactorings that Fowler enumerates, other than that they all seem to be sound practice. I'll learn more as I put the tools Fowler has provided into use.

One thing that I do know is that I couldn't have done this small set of refactorings without the catalog at my side. I've never been good at memorization (I keep a to-do list for even obvious things like "Eat," "Do Laundry," and "Sleep"). When I sat down to put some elbow grease into the open source Thingamablog (a cross-platform desktop blog editor), I was overwhelmed. The code was a mess (no offense to the authors; to their credit they did come up with a really good Swing interface). There was a mix of Java 1 arrays (String[]) and Java 2 data structures (Vector / ArrayList), there was an odd structure of all the domain logic existing in a nebulous nether-region between the UI and the Domain (and, no, not in controllers.. at least in the modern MVC sense of the word) and there were single letter parameters.

Small (baby) steps

So, I fumbled around the code for about an hour trying to understand the code and its interactions. I was making big design decisions in my head and was really quickly overwhelmed. This was going to take days. I had a deadline that wouldn't accept days. How was I going to do this?

Then, I remembered the advice back in Chapter 2 of Refactoring. Refactoring is about small steps that lead to some eventual larger goal. I didn't need to think about the big things; I didn't need to take days in order to improve the design. I just needed to take baby steps.

Baby stepping

Here's a list of the steps I took. Something that I want to note is that through every step of the way, Refactoring was an amazing resource. I leaned on the Mechanics sections in the catalog. Also, there won't be a lot of code because there is too much. I will provide some UML Diagram where appropriate.

Step 1

I was flying a little blind, so I just chose a random[1] class to start with. I picked the BlogEntry class. I didn't understand what was going on, so I started with some simple name refactorings. I did a couple field renames, quite a few parameter renames and a method rename before I felt like I was comfortable with the class.

Step 2

There was one more refactoring that I could do to the BlogEntry class. There were getters and setters on a collection (Categories). I used the Encapsulate Collection (208) refactoring to hide the implementation of the collection.

Step 3

So, then I wanted to branch out from the BlogEntry[2] and see what classes used it. I found the BlogEntryContainer had a reference to BlogEntry (no surprise there). In the method that I first was shown by IntelliJ I saw a large switch statement that was using object types. I identified this as a code smell and set out to refactor it.

The Replace Conditional with Polymorphism (255) is one of my favorite refactorings. It is a cool, practical application of polymorphism. It is also the entire basis for the Anti-If campaign. While I'm not necessarily an advocate for replacing all ifs with polymorphism, I am definitely anti-switch and would prefer to use polymorphism instead of switches.

Fortunately, the hard work was done for me in this case. The polymorphic classes already existed! All I needed to do was create a method on the abstract class and implement it in all of its children. It worked like a charm. UML Diagrams follow to illustrate.

And it collapsed a ~50 line switch statement into 1 line of code. It always makes me happy to reduce complexity and code at the same time.

Step 4

Next I noticed that there were some preferences (about 8 of them) that were being maintained as private fields in the BlogEntryContainer class. The fields were always requested data from the TBWeblog class (feature envy). I put to work the Move Field (146) refactoring. I moved the eight fields one at a time.

Step 5

Moving the fields didn't fix the problem, though. There was still feature envy in several methods. I used Move Method (142) to move 5 different methods over to the TBWeblog class.

Step 6

After moving the fields over to TBWeblog class, I noticed that there were some more refactorings that I could put to use. I renamed some methods, renamed some parameters, consolidated conditionals and decomposed one of the conditionals. These all were relatively small refactorings that changed the entire look of the application.

Final Words

After all of these steps (and about 3 hours of actual refactoring) I was surprised to learn just how effective all of this was. It reminds me of something I read/heard this summer.

Never let best get in the way of better.

I wish I remember who said that (it very well may have been Jon Fuller) but it is a great quote for programmers. It means that you should never discard making something better because you can't make it the best. For example, if there is a large refactoring that will make the code better but you don't have time to do it right now (maybe even ever!), don't let that stop you from doing it the right way in small increments. That could mean that if you realize a bad design decision (it happens) then you should follow the better design from now on at the least and incrementally improve the existing system.

This week, I almost let that small nugget be forgotten. I almost asked for an extension in order to get this "refactored properly" but instead I went with the "better" instead of "best" and I am still very satisfied with the improvements that I made to the code. There is still a lot of work to be done but that'll have to be for another day, another time.

[1] I actually tried to pick something in the domain model because that is often where class interaction gets complicated. Unfortunately the author chose to follow the "dumb" domain class model where little - to - no logic exists in the domain classes.

[2] I originally wanted to try to pull some domain logic into the BlogEntry class but I never go to that point in the refactorings. It could definitely be done, though.