Timmy Von Heiss
Timmy Von Heiss

Reputation: 2218

how can I DRY this controller code?

These are actions in the same controller:

  def world
    @title = 'World'
    @regional = @articles.world.paginate(page: params[:reg_page], per_page: 6) 
      respond_to do |format|
        format.html { render 'index_region' }
        format.js { render :file => "articles/latest.js.erb" }
      end  
  end   

  def politics
    @title = 'Politics'
    @regional = @articles.politics.paginate(page: params[:reg_page], per_page: 6)
      respond_to do |format|
        format.html { render 'index_region' }
        format.js { render :file => "/articles/latest.js.erb" }
      end 
  end

  def sports
    @title = 'Sports'
    @regional = @articles.sports.paginate(page: params[:reg_page], per_page: 6)
      respond_to do |format|
        format.html { render 'index_region' }
        format.js { render :file => "/articles/latest.js.erb" }
      end 
  end

As you can see this code is very repetitive. This goes on for several more entries. Is there a way to DRY this up? Is it possible or advisable to create some kind of block function?

Upvotes: 0

Views: 56

Answers (2)

Shamsul Haque
Shamsul Haque

Reputation: 2451

Edit the controller:-

  def custom_method
    @title = params[:title]
    if @title== 'World'
      @regional = @articles.world.paginate(page: params[:reg_page], per_page: 6) 
    elsif @title = 'Politics'
      @regional = @articles.politics.paginate(page: params[:reg_page], per_page: 6) 
    elsif @title = 'Sports'
      @regional = @articles.sports.paginate(page: params[:reg_page], per_page: 6) 
    end
      respond_to do |format|
        format.html { render 'index_region' }
        format.js { render :file => "articles/latest.js.erb" }
      end  
  end

Pass the title as parameter in the link.

In routes.rb:-

get 'news/custom_method' => 'articles#custom_method'

This will generate the link as news_custom_method_path

Use it in View as:-

<%= link_to "World", news_custom_method_path(id: articles.id, title: "World") %>
<%= link_to "Politics", news_custom_method_path(id: articles.id, title: "Politics") %>
<%= link_to "Sports", news_custom_method_path(id: articles.id, title: "Sports") %>

You can rename the custom_method according to your choice

Upvotes: 1

Ninigi
Ninigi

Reputation: 1311

I don't want to give any wrong ideas here, without digging deeper into your project, but speaking from my experience (and apparently also in DHHs opinion) it is always a good idea to stick to RESTfull resources in your controllers.

Basically what your politics/sports/world methods do, is showing more specific instances of articles, right? So what you could do, is (sticking to REST) have an index method like this:

def index
  category  = params[:category] || "all"
  @title    = category.humanize
  @regional = @articles.send(category).paginate(page: params[:reg_page], per_page: 6)

  respond_to do |format|
    format.html { render 'index_region' }
    format.js { render :file => "/articles/latest.js.erb" }
   end 
 end

Rendering the same templates in all actions is a very strong hint that you should have them in one method. You will have to rewrite your other code to use the articles?category=sports resource, instead of articles/sports of course, but imo it is totally worth it.

Upvotes: 1

Related Questions