Posts [ 1 to 20 of 22 ]

Topic: DRY and RESTful Controllers

As I fall deeper into the Rails rabbithole, I find myself violating the DRY principle quite a bit, especially in my controllers. The problem stems from the fact that I rely so much on script/generate scaffold_resource... the result?

Now I am finally beginning to understand why all these Rails vets tend to shun generated code... it's a great reference for best practices but it's no substitute for writing your own methods DRY from the start. More and more I am seeing that I need to let Rails natural inheritance work for me.

It occured to me that the bulk of my CRUD actions really belong at the application controller level,, After some fast Googling I saw that I was not alone...

Re: DRY and RESTful Controllers

I also found myself doing a simlar refactoring. I created a Resources controller (subclass of application controller) which the other CRUD based controllers could be a subclass of. This controller has a few before filters to handle permissions and set up the models. It worked marvolously. It made the app more secure and removed some duplication.

However, I caution you, be careful with this! It doesn't apply to every RESTful designed application. I consider it premature abstraction if it is applied too early. Instead, extract this behavior through refactoring when you see the duplication.

The reason being is because you can't tell how much behavior should be moved into this resources controller until the app is built. If you do it prematurely you run the risk of extracting too much of the behavior into the controller to the point where it becomes difficult to customize the behavior per controller and can lead to code which is worse than having the duplication.

Re: DRY and RESTful Controllers

I struggle with that too. It's hard to consistently refactor as you are coding.

One thing that really helps me is good test coverage. This way, when you refactor you instantly know whether or not you broke something. The refactoring I did previuosly with the Resources controller would be impossible without tests. This is because it involved so many controllers across the application and there's no way I could check everything after every little change. The tests reminded me when I was trying to refactor too much at one time.

Re: DRY and RESTful Controllers

As I build my first controllers I already see much duplication, especially when dealing with RESTful resources. I am considering utilizing this example as an application helper (the author seems to encourage it in application controller but Im not sure thats the best move)

Re: DRY and RESTful Controllers

No problem BIGtrouble77

I'm actually surprised more people don't do this. Having 20 different controllers with the same 7 actions just seems obscene to me. (thats 140 repeats!)

As a side note, the code in that link is little beyond my comprehension. I need to study it a bit more so I can bend it to a more RESTful standard. Unless somebody here has already done something similar for REST, this will become a pet project of mine.

Re: DRY and RESTful Controllers

UPDATE: This is becoming both frustrating and educational for me.

The way I see, it REST controllers should be pretty simple to refactor.. the CRUD is almost always identical and cases when it isn't can be easily overridden. You basically have the same exact methods/actions with the model name sprinkled throughout, so its really a matter of setting the model name dynamically. Easy right?

This is where we leave the glorious land of REST and return to the days of yore, when actions were explictly programmed into redirects.. I mean it's great there is a respond_to block in there and all, but how is this even remotely RESTful if the routes and conditions are not specified? No wonder my view bombs! The action should look more like this (pardon my butchery):

Of course this doesnt work either because I haven't yet learned how to parse method_names in different situations. Put an @ in front of a method_name and *POOF* it's an instance variable!

Argh... so much voodoo going on here!

I'm not letting this setback discourage me though. Sure it's holding my production up right now, but it's also an opportunity to learn how my code is working, which will just make me a better programmer at the end of the day.

PS- One thing I really can't figure out from the original code (or find much reference on) is that params_hash dealey..... what is going on there exactly??

The current_model getter and setter methods is where the magic is happening. This is using an instance variable with the same name as the model itself. For example, in the Products controller, it will use an instance variable called @product to store the name of the model.

Re: DRY and RESTful Controllers

And now for the lecture.

My theory is that there is real duplication, and then there's fake duplication. Just because the code looks the same, doesn't mean it's real duplication.

Instead of determining duplication on how the code looks line for line, base it on the behavior of the code and how changing it should effect the other "duplication". If changing one part of the duplication should change the others as well, then that is real duplication. If not, then I consider it fake duplication.

If you are building the site interface first (which I recommend) then there is likely not 100% real duplication across the controllers. First of all, you don't need all 7 CRUD actions for every controller. Some don't need an index, they just need the create/update/destroy actions. In the "index" and "show" actions you likely need to set up more instance variables than just the model. You may not want to always redirect to the same action (index/show) after creating/editing a page. You might want to change the flash message, or create two models at a time instead of one, etc.

In other words, you need flexibility in the controllers. If every controller action is exactly the same, ask yourself if you are building the site interface first. I'm finding controllers often have differences. This is the true reason why scaffolding and generators are bad. See the post by core member Jamis Buck Scaffolding's place for more info on this subject.

Yes, you can override the 7 CRUD actions in the subclasses, but you need to override the entire thing. But if you just want to change one little thing (say the flash message or redirect) then overriding the entire action becomes silly because you are back with the duplication that you tried so hard to avoid. You can apply some more refactorings to remove the duplication further, maybe add callbacks for determining the flash message and redirect location, but this will likely result in a big mess. Why? Because the duplication is fake, it is deceptive. You need more flexibility in the controller actions.

This is why I cautioned you in the beginning about how much you move into this abstract controller. IMO, abstracting all 7 actions is going too far. You may be wondering how my controller looks then? I'll show you:

# In an attempt to refactor the permission handling and fetching of models in most controller actions# this abstract resources controller should be used as the superclass for all controllers that# are managing a model with basic CRUD operations. The controller should have the plural name of the model.class ResourcesController < ApplicationController

def authorized? permission_actions.all? do |action| current_user.can?(action, model_name, current_model) end end

def permission_actions case params[:action].to_sym when :index, :show then [:view] when :new, :create then [:view, :create] when :edit, :update then [:view, :update] when :destroy then [:view, :destroy] else [] end end

Notice how every method in here is protected? There are no actions here, it is just before filters. Normally I would not perform this kind of abstraction, but the complex permission and authorization handling called for it. The model needs to be created before the authorization can take place because the user is asked if he has permission to alter it.

With this abstract controller, take a look at what a typical subclass looks like:

In this case the @user instance variable is already set based on the before filters. All that needs to happen is the saving and setting of other attributes in the create/update actions. The flash message, redirect, etc. needs more flexibility (fake duplication) so that is why it is not extracted into the ResourcesController.

Re: DRY and RESTful Controllers

That is my blog over at http://geekonomics.blogspot.com and I just wanted to share some experience and thoughts on this topic since I have used the code and adapted to change over the past few months.

1) I found that the number one way I was using it was to bootstrap controllers during development. Unless you have a really super simple system, your controllers are going to need to change and be unique over time.

2) Edge Rails and now 1.2 has had a better replacement for this type of bootstrapping for some time now. script/generate scaffold_resource ... This method usually saves more typing that CrudController ever will.

3) It can be dangerous! What if you dont actually want a destroy action for one of your controllers? You then need to remember to either overwrite this method or un-subclass that controller.

Re: DRY and RESTful Controllers

Actually, that makes a boatload of sense, ryan

Instead of a master class to lump all methods together, a more modular subclass of actions would assist me much more to control related behaviors. Thanks for giving us a peek in the toolbox. This stuff should be in a book (hint, hint)

Also many thanks to jakehow for dropping in and keeping my out of trouble

Re: DRY and RESTful Controllers

Re: DRY and RESTful Controllers

Okay, been really digging into your examples here and am pleasantly surprised how much I understand. The lights in my head are switching on!

All the same, you are using some curious goodies. One in particular is

find_or_initialize

This seems to be a recent addition to Rails and not much info is out there (AWDWR mentions it once and google aint much help either) From what I can gather, this acts as both a create and update method, but I am not sure how it is triggered.

Examples:

1. Will the URLs models/new and models/1;edit point to this action automatically?

2. In my views should I still use form_for(:model, :url => models_path)?

This method is set up as a before_filter which is called before many different actions. This is so it sets up the instance variable before calling the primary action (show/new/create, etc.).

For example, when the "/users/create" action is called, the params[:id] will not be specified so it will make a new User model and and assign it to an instance variable of "@user".

When the "/users/edit/1" action is called the params[:id] is already established so it will find a user with id 1 and set it to the @user instance variable. This is so the actions in the controller subclasses don't have to set this variable. I also do this because I need to authorize the user later on, and it needs to have a model to determine if the user has permission to access it.

Re: DRY and RESTful Controllers

This isn't shown. It's just a method in the ApplicationController which returns true/false depending on if the user is logged in or not.

weelkoh0 wrote:

I can understand what 'permission_actions' method is doing. However, what does the .all? (in permission_actions.all?) do? Also, I could not find 'current_user.can?' in Resources Controller.

The "all?" method is provided by Ruby on an enumerable. Basically it loops through each item (each permission action) and runs the code in the block for each one. It then returns true only if each loop returns true.

The current_user.can? method is a method in the User model which handles the permissions. Unfortunately it's too complicated for me to go into detail here.

weelkoh0 wrote:

Does Resources Controller have a corresponding resources database table?