This week I spent some time skimming through Refactoring: Ruby Edition and also put some more refactoring into Redmine. The book, unfortunately, is just a rehash of Fowler's original but it does have some nuggets of information. The continued refactoring of Redmine was both interesting and intriguing. I had a few tough decisions to make but I think that, in the end, the factoring of the code was much better than the original.
Refactoring: Ruby Edition
Jay Fields released this book back in the fall and I was really excited to grab it. It was a book specifically about one of the most interesting topics (refactoring) and in one of the most interesting languages (Ruby). I was really excited.
Unfortunately, I found that I've already read most of this book. All of the examples are, of course, in Ruby instead of Java but the examples are almost exactly the same. The catalog has, for the most part, the exact same content (I'll get to some of the differences in a second). Despite all of this, though, it is still a great reference and a great read if you haven't read Fowler's book already.
The first chapter I had already read and practiced. I did this in Chicago at 8th Light with Doug Bradbury. We went step by step through the chapter and recreated the chapter in code. It was fun.
The chapter on testing was significantly different; Jay talked about Test::Unit (and, briefly, RSpec) instead of JUnit. He gave a good overview and stuck to the spirit of Refactoring.
The catalog had a few differences that I should point out (also, I've only read half of the catalog, more next week on the other half). Specifically, Jay expands a few chapters (like Extract Method) to include several Rubyisms. This is nice because algorithms to refactor Java do not always translate completely to other languages (like Ruby).
Jay also introduced a new pattern that is very Rubyist but also very powerful.
Refactoring Redmine, Part 2
This week I'm revisiting Redmine. I really like working with this project because it
- Has a lot (and I mean a lot!) of tests. This makes my life easier.
- Has well factored code. This makes me really think about the refactorings. In situations such as the Android function, I don't need to think about the refactorings because the code is so bad that almost anything I do is an improvement.
- Is written in Ruby and Rails. I just like these two technologies.
So, once again, I set off on a random controller and attempted to refactor. I selected the Project Controller and refactored the first method I saw (ProjectController#add).
Extract Method. Nothing special about this one.
Extract method and add guard. This is my really cool Ruby refactor that I love to use. Seems to improve the code a lot.
Renaming to make the code more clear.
Rename Method to follow Ruby convention.
Move Method. The comments already stated that they wanted to do this. I agreed. I moved the method to the Project class. I was faced with a tough decision here, though. I could have NOT passed in the parent_id because the params object is accessible in the Model, as well. However, I looked at the method and accessing the params object is not a key element to the method, but is instead just a way to access a parent_id. So, I passed it in as a parameter.
Move Method Block. I moved this code outside of the extracted method because it wasn't really doing what the method said it was doing.
Extract Method. Nothing really special here.
Using guards instead of an if block. If you look at this and think that this wasn't positive, just wait. I end up agreeing with you.
Extracting assignment to caller. I did this so that I could try something. I end up regressing some of this. I don't think that this was really positive or negative.
Using a guard instead of an embedded if block.
Reverting my guards and if block changes. The three statements that had the if block just looked ugly. I don't like the if block (too much nesting in this method) but we're still working.
Reverting the pull out here. Where I wanted to go wasn't going to happen so I reverted part of that change.
Getting rid of that nasty if/else block. I don't like the "and return" here.
Got rid of that nasty "and return" by extract method and add guard. Didn't know exactly what to call the method, yet.