Independent Study :: Refactoring Redmine

This week I finished reading Refactoring and started to apply some more basic refactorings to Redmine. The end of Refactoring was as interesting as the beginning and Kent Beck's final chapter was really insightful. The refactorings that I applied to Redmine were more good practice at using simple steps to improve the design. Mostly, I focused on composing methods.

Refactoring, Final Chapters

The final chapters of refactoring were interesting. William Opdyke's research into refactoring and his chapter on it had several interesting pointers and thoughts on the practice of refactoring from a research perspective. One really interesting nugget of knowledge is that refactoring is a play on the mathematical term factoring.

A factoring is a way to write a mathematical equation: factoring is applied to equations in order to increase the readability of that equation in that context. For example, Newton's second law is famously known as F = ma. This is correct, but is a factoring that makes it easier to understand in the context of which it is presented (the general public). Newton actually described F = d(mv)/dt or as the rate of change of an object's linear momentum.

Kent Beck's chapter was also very good. Kent expounds the importance of refactoring as a (very powerful) tool in a developer's toolkit. However, after reading a whole book about refactoring and how it improves your code, your efficiency, the world, the universe and everything, Kent says that one of the most important aspects about being good at refactoring is knowing when to stop. Developers will often have an end goal when they refactor and that is wonderful. However, sometimes that end goal becomes too difficult, too costly, too ideal to accomplish and the developer has to make a decision: keep the refactorings or toss them. In general, the refactorings will have improved your code but sometimes the best thing is to just toss them (especially if you were regressing the code in order to make something magical happen).

As I read Beck, I wish I had the confidence to know when to stop and when to keep going. After reading that chapter and going through the refactorings to Redmine, I found myself asking "Is this too much? Or have I just barely scratched the surface?" It was difficult to judge where in the spectrum I was at any given time. Hopefully the rest of my experiences with this independent study will give me some insight into this question.

Redmine

Switching gears, I went to a Ruby on Rails project called Redmine. Redmine is an open source project management suite. I like Ruby. I really wanted to learn how to effective refactor in a dynamic language such as Ruby (hint: its not that much different, you just need *really* good tests). Redmine was a good fit because it had a significant test suite for a significant sized Rails project. What I learned was that Rails doesn't lend itself as easily to the spaghetti code monster that was the Android function, but refactoring can improve the readability of code just as much.

Step 1

http://github.com/hammerdr/redmine/commit/8ea55533023831dc456effc6ff404a47198af4dc

Extracted a Method.

Step 2

http://github.com/hammerdr/redmine/commit/85784bed9fd7009523426d2b5e7b4f2a36589f79

Extracted a Method.

Step 3

http://github.com/hammerdr/redmine/commit/6f7e8c06e96bcd78d9ff2d936ef8abafd2b0fabf

Split a Method. :find_project was doing more than "finding (a|the) project", it was finding a board associated with the project. So I split the method.

Step 4

http://github.com/hammerdr/redmine/commit/954b5a892b20c09a04fa57bd064498123dfa8d63

Extracted a Method. Moving to a different area of the code (was satisfied with the other file), I found the account_controller. There was some duplication so I extracted a method.

Step 5

http://github.com/hammerdr/redmine/commit/f0a6eb6ead70cc4a34dff770bdcadab3eaa85f82

Removed redundant code (Rubyisms).

Step 6

http://github.com/hammerdr/redmine/commit/61ce6a5867ea3182c7a590c103c05f5f7cf31da1

Then I had some trouble. So I talked myself through it and extracted a couple methods to make sure I was sane while I was doing it. My stream of consciousness thoughts are written in comments.

Step 7

http://github.com/hammerdr/redmine/commit/8c8bc970442bc22b26dc5bbb7def854a2ab377a8

Removing redundant code. else -> if can be merged to elsif

Step 8

http://github.com/hammerdr/redmine/commit/5af748768179630e2eb3f8299ddca03045cf304a

Adding guards. I could add guards, remove return statements and make the code read better. Yes, yes and yes.

Step 9

http://github.com/hammerdr/redmine/commit/b2f388aa5a54b6099547ce12e740a8e4b5e330f1

Extract method and Add Guard. This little combination was just cool. I took a long if statement, extracted it to a method with a good name, and used a guard instead of a if block. Much cleaner.

Step 10

http://github.com/hammerdr/redmine/commit/614e83eed41483df7a81380108580a3e5db85ae8

Encapsulating. Just something I forgot to do correctly the first time.

Step 11

http://github.com/hammerdr/redmine/commit/931f1717d71e976109af62edf60a50da0079c721

Add Guards. These guards are more expressive than the guards that originally existed. Cool. I like expressive.