I'm really new to rails, so i programmed some stuff and today i read
some things about skinny Controllers, fat models.
My Controllers are really fat now. So i'm asking myself how can i
shrink my controllers and move the code to the models, especially in
fact of REST e.g. in focus on error codes?
code example:
# POST /tasks
# POST /tasks.xml
def create
@authorized = false
@addsubtask=false
@task = Task.new(params[:task])
if(@task.parent_id != nil ) #wenn parent id nicht leer ist
ueberpruefe ob Rechte fuer Uebertask da sind
#if ( current_user.is_owner_of? Task.find(@task.parent_id) ) ||
( current_user.is_moderator_of? Task.find(@task.parent_id) )
if ( current_user.has_rights_for_task? Task.find
(@task.parent_id) , ["Owner","Moderator"])
@authorized = true #Rechte da
@addsubtask = true
else
@authorized = false #Rechte da
@addsubtask = true
end
end
if ( ( (@authorized == false ) && (@addsubtask == false) ) ||
( (@authorized == true ) && (@addsubtask == true) ) )
respond_to do |format|
if(@task.priority == nil)
@task.priority = Priority.find(2)
end
@task.accepts_role 'Owner', current_user
if @task.save#Task an
if @addsubtask@task.move_to_child_of(@task.parent_id)
end
@task.recalculate_progress_recursive
flash[:notice] = 'Task was successfully created.'
format.html { redirect_to(@task) }
format.xml { render :xml => @task, :status
=> :created, :location => @task }
else
format.html { render :action => "new" }
format.xml { render :xml => @task.errors, :status
=> :unprocessable_entity }
end
end
else
respond_to do |format| #BENUTZER HAT KEINE RECHTE subtask
anzulegen
format.html { render :file => "#{Rails.public_path}/
401.html", :status => :unauthorized } #401 page laden
format.xml { render :xml => @task.errors, :status
=> :unauthorized } #statuscode bearbeiten
end
end
end
You probably should extend your code by creating a module that deals
with many of the conditions you are trying to handle. Then all you have
to do is include the module in the controllers that require the methods.
module Foo
def self.method_one
end
def Foo.method_two
end
class << self
def method_three
end
end
end
You can do it a number of ways as my example shows. The module goes in
your lib folder. You include it by doing:
class YourController < ActionController::Base
include Foo
Yes. I use make_resourceful, which is similar. Note, however, that
neither this or Alpha Blue's suggestion really addresses the question of
making controllers skinny.
The OP's controller is beyond fat into morbidly obese. To fix, read
some good stuff about refactoring. The short version:
0. If you haven't already got tests and Cucumber features, write them
now so you can make sure you're not breaking anything. And then get
into the habit of writing tests first.
1. Find a coherent block of code.
2. Extract that into a separate method.
3. Move that method into the model.
4. Lather, rinse, repeat.
Basically what you will be doing is ‘extract method’ and shifting the reponsibility
away from the Controller. The post reponding to this method about creating module with methods is valuable - but to really get skinny controller/fat model one needs to put the calls
onto the model level.
The first step is to Think about you model having methods encapsulating your
if logic
authorized? and add_as_subtask? now one of these may be about a specific task - then
it would be an instance method - one of these may be about a Tasks in general - then that
is a good canidate for a class method.
One thing i like to do is to make a ‘build’ (and build!) class method that encapsulates a bunch of creation logic in the model.
;
ie
class << self
def build(params)
end
end
Ok, i will consider that.
Another question: how should i capsulate the rendering of unauthorized
access?
I will present the unauthorized error in varoius formats like html,
xml and js, so whats the best way to do that?
To fix, read
some good stuff about refactoring. The short version: