Is this too DRY, am I going overboard?

I've been drying some code, one of this refactors is as following:

I have 3 controllers ( ConstructionCompanies, RealEstateCompanies, People) all of which had the following pattern:

class ConstructionCompaniesController < ApplicationController
before_filter :correct_user, :only => [:edit, :update]

private
   def correct_user
      @company = ConstructionCompany.find(params[:id]) 
      if(current_user.owner != @company.user)
        redirect_to(root_path)
      end
    end   

class RealEstateCompaniesController < ApplicationController
  before_filter :correct_user, :only => [:edit, :update]
...

private
   def correct_user
      @company = RealEstateCompany.find(params[:id]) 
      if(current_user.owner != @company.user)
        redirect_to(root_path)
      end
    end   

As you can see the correct_user is repeated in each controller.

So what I did inside I helper that is included for all of them I created a method:

def correct_user_for_seller_of_controller(controller)
    #"User".classify wil开发者_如何学Gol return the class User etc.
    @seller = controller.controller_name.classify.constantize.find(params[:id])     
    redirect_to(root_path) unless (current_user == @seller.user)
  end                            

Know inside each controller I have:

class ConstructionCompaniesController < ApplicationController

  before_filter :only => [:edit, :update] do |controller| correct_user_for_seller_of_controller(controller) end           

class RealEstateCompaniesController < ApplicationController

 before_filter :only => [:edit, :update] do |controller| correct_user_for_seller_of_controller(controller) end           

I like the fact that is DRY now, but the problem is that it seems a little to complex for me, hard to understand. Did I went too far ?

Add the correct_user method to the ApplicationController class.

class ApplicationController
  def correct_user_for_seller_of_controller
    #"User".classify will return the class User etc.
    @seller = controller_name.classify.constantize.find(params[:id])     
    redirect_to(root_path) unless (current_user == @seller.user)
  end  
end

In your controllers use the new method as a filter method:

class RealEstateCompaniesController < ApplicationController

 before_filter :correct_user_for_seller_of_controller, :only => [:edit, :update]

end

It's definitely good to clean that up. I think you may have made things slightly more complex than necessary, though, what with all the Proc whatnot.

If the filter is being run on an instance of the controller, then there's no need to pass the controller to itself. Just call controller_name within the method and you're good to go.

I'd personally DRY it a bit more and move the filter to a superclass (or AppplicationController).

The method itself could also definitely be simplified. Use some introspection for example.