Topic: Refactoring on Rails: Multiple Scopes in Controller
If you hang around the refactoring crowd long enough, you may hear the term bad smells in code. Smelly code works, but the code itself is poorly implemented. If you keep coding without removing the bad smell, it will only get worse and become harder to fix in the future.
When we find this stinky code, how do we fix it? Because the code is already satisfying the requirements, refactoring is the perfect solution. Refactoring, as you may recall from the previous article, is improving the design of existing code without changing its functionality. Therefore, each smell has its own set of recommended refactorings that take care of the smelly situations.
Martin Fowler lists several bad smells in his book. By far the most popular smell is Duplicate Code. The other smells/refactorings in the book are equally generic, but we can take this concept and create Rails-specific smells and refactorings. This article will do just that by defining one smell and one refactoring to solve it.
Smell: Multiple Scopes in Controller
If a controller has a large number of actions, it may be suffering from multiple scope-osis (sorry, I couldn't resist). This problem can usually be spotted by simply scanning the method names. If you see a group of actions that seem to go together, you should consider doing the Extract Controller refactoring (see below) to move the group of actions into its own controller.
Let's take a look at an example. Here we have a controller with a list of actions:
This controller obviously has more than one scope. How many do you see? I see three. One scope handles the displaying, creating, and editing of the account model. Another scope handles user login/logout. The third scope manages the account's addresses.
This makes the controller unnecessarily large which will only get worse as we add features. The four address actions should be moved into its own controller. The login/logout actions probably should too, but that's not as critical as it is only two actions.
Refactoring: Extract Controller
This refactoring involves creating a new controller from a portion of an existing controller. To summarize: a new controller is created, the desired actions are moved to this new controller along with their views, partials, helpers, etc., and the old actions are set to redirect to the new actions. If we apply this refactoring to the "addresses" scope in our Account Controller, we get this:
Much cleaner. If we try to do this all at once, we are bound to break something. This is why refactorings are broken down into little steps. Here are the steps for this refactoring:
1. Generate a new, empty controller
2. Copy and paste all action methods you wish to extract from the original controller into the new controller
3. Copy the necessary view, helper and partial files to the new controller.
4. Copy any necessary filters or private methods to the new controller.
5. Copy the tests which apply to the actions over to the new controller test case and run them. This will tell you if you forgot to copy anything else.
6. Change the actions in the original controller to redirect to the matching actions in the new controller. Update the tests accordingly.
7. Remove the views, partials, helpers, filters, private methods, etc. from the original controller which belonged to the extracted methods and are no longer necessary. Constantly run the tests to make sure nothing breaks as you do this.
8. For every existing partial used by both controllers, decide which controller should "host" it (or place it in a "shared" directory) and update the views to render the partial from that location. Remove the unnecessary, duplicate partials.
9. For any duplicate helper methods that exists in both controllers, move them into the ApplicationHelper module.
10. For any duplicate private/non-action methods that exist in both controllers, move them into the ApplicationController.
11. Change the links in all the views to use this new controller.
12. If you do not need to support external links, remove the actions in the original controller that are just redirecting to the new controller.
If you can think of any other smells or refactorings, consider writing them down and creating your own Rails Refactoring Catalog. You may also want to post a tutorial here describing the refactoring.
One thing interesting to note, some refactorings can lead directly to other smells/refactorings. For example, this refactoring had you move the common code into ApplicationHelper and ApplicationController. If either of these get very large or contain many methods that are only used by a couple controllers, that would be considered a smell. It would be interesting to see what kind of refactorings can solve that problem.
Now go clean up your controllers.
Last edited by ryanb (2006-11-16 12:31:36)