I'm running about half a week behind on my posts but I hope to be able to catch up this week. For this entry, I have decided to refactor one of my own projects. I actually attempted to dig into my dark, dark repositories of code from Freshman and Sophomore year but the code that I saw horrified me so much that I decided to refactor a more recent project, instead. I may or may not visit those projects next week.
I quickly learned that it is much easier to refactor my own code even months after I had written it. This is because I quickly and easily realize mistakes that I made in code: I have certain habits and tendencies. I can go down a checklist in order to resolve those mistakes I commonly make. Here's that checklist (and, as I learn more, this should grow):
- Are there methods that belong in another class? (Feature Envy). If so, move those methods to the other class via Move Method. I tend to be single-minded and attempt to complete the functionality in a single location rather than thinking about the implications of the location of the code.
- Is the code DRY? If not, use Extract Method to remove duplication. I can sling code, sometimes. Usually it is when I'm "on a roll." Turns out, however, that whenever I'm on a roll I tend to do bad things: duplicate code, forget to write tests first, create feature envy (see above), etc.
- Is there code that really belongs in another class? I'm usually reluctant to break code out into a class unless I'm refactoring or starting a new feature. When I'm refactoring, I really need to investigate the responsibility of the class. If it has more than one, break the offending code out into a new class.
- Is my coupling too tight? I fall into the trap of high coupling too often when I'm slinging code (even if I'm doing TDD). The problem is that I approach code very piecewise. When I'm refactoring, I need to look at the bigger picture and the ability for the code to flex, not break.
- Am I reinventing the wheel? I do this one way too often. Sometimes when I am hacking away at code I end up doing something that is already done in another part of the codebase or libraries.
- Am I encapsulating the data correctly? I'm very guilty of this. Especially with collections. I should be encapsulating the internal representation of the data. This also goes for self encapsulation, though I'm not a True Believer in that.
Then, of course, I would go through the normal code smells run down. Is this method too long? Is this field named correctly? Does this constructor make use of the default constructor? Etc. This checklist, though, is a very targeted means of improving my code. I'm going to try to go through this checklist whenever I write a feature on any codebase from now on. I think that it'll improve my code greatly in a relatively short amount of time. Yay!
So, with my checklist in hand and my project selected, I set out. The end result, I feel, is really good. I broke it up into two parts. The first part is my refactorings. The second part is additions of features to the code.
- Matching Objective-J coding standards
- Refactoring to OJMoqAssert
- Moving comparisons to OJMoqSelector
- Making OJMoqSelector control finding selectors
- Making __ojmoq_fail() a method on OJMoqAssert
- Got rid of redundant __ojmoq_findSelector
- Got rid of an unnecessary global
- Encapsulating data representation and renaming ivars
- This was to make the equals: message behave more sanely. Before a == b did not imply that b == a, which is inconsistent with the definition of equality. It made it difficult to wrap someone's head around it. So, I changed it so that a == b => b == a. Now, the numberOfArguments logic is contained in the "find:by:" selector which states that A can have arguments and still match B which has no arguments, as long as the names are equal.
- Reuse of method that I created
- Getting rid of extraneous code
- Final clean up
There are two big refactorings in this set. The first is the extracting of assertion-esque code to the OJMoqAssert class. It only has class methods but the abstraction allows for easy testing and better readability of the code. The second is the removal of feature envy from OJMoq and putting the relevant code into OJMoqSelector. It made both look better. A pivotal commit is the one with the long message. It was where I realized that having
OJMoqSelectorA == OJMoqSelectorB not implying
OJMoqSelectorB == OJMoqSelectorA was a bad thing.
This part may not be interesting from a Refactoring point of view. However, as Martin Fowler points out, refactoring is often the result of wanting to add a feature to a code base. So, I wanted to see how much refactoring would be influenced by my desire to implement new features. It was definitely interesting and a more "real world" approach to refactoring. One thing that I noticed is that I had a decent set of test cases but the refactoring still introduced a few bugs that I had to fix down the road.