skinny Controllers, fat models with REST?

Hi,

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

greetings
LeonS

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

  def create
    ..
  end

end

I personally use ResourceController,
http://github.com/jamesgolick/resource_controller, for controllers
using the REST-pattern, but
http://github.com/josevalim/inherited_resources seems very nice too.

/K

Kristian Hellquist wrote:

I personally use ResourceController,
http://github.com/jamesgolick/resource_controller, for controllers
using the REST-pattern, but
http://github.com/josevalim/inherited_resources seems very nice too.

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. :slight_smile: 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. :slight_smile:
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.

/K

Best,

Hey Leon!

very fun!

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

  1. 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. :slight_smile:

  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.

This is a wonderful post.

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?

:status => :unprocessable_entity

I use the above - I don’t explain why unless we are talking html and it’s for a users benifit. I don’t like to give potential crackers help.

I think i found a very nice solution:
http://techblog.floorplanner.com/2008/03/11/putting-http-status-codes-to-use-with-rails/

so what do you think about that?

Elegant. Thank you!