質問

Im stuck trying to find a way to refactor this ugly controller

  def video_games
  @video_games_released = Item.video_games.released.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
  @video_games_coming_soon = Item.video_games.coming_soon.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
  @video_games_unknown = Item.video_games.unknown.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
end

def movies
  @movies_coming_soon = Item.movies.coming_soon.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  } 
  @movies_released = Item.movies.released.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
  @movies_unknown = Item.movies.unknown.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
end

def tv
  @tv_coming_soon = Item.tv.coming_soon.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
  @tv_released = Item.tv.released.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }
  @tv_unknown = Item.tv.unknown.group_by { 
    |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
  }  
end

I want to get rid of the duplication espcially my group_by methods

I've tried going into the model and creating a method

 def group_by_month
   self.group_by { 
     |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
   } 
 end

ive tried scoping it but nothing seems to be working

I'm no rails expert and really trying to learn how to refactor code and keep things dry

役に立ちましたか?

解決 2

You can use groupupdate gem it will give you this functionality on the database level

That was the easy way

If you want to do it in a way to learn more about rails

you have an obvious block that you use over and over again

{ 
     |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()
   }

you can save it and call it when you need it

date_group = lambda { |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()}

and when you need to use it you can do

Item.tv.coming_soon.group_by(&date_group)

you may want to save this block in some place that you can easily access

他のヒント

so what i did to refactor this code with the help of khaled_gomaa suggestion was

i created a presenter in

app/presenters/items/index_presenter.rb

module Items
class IndexPresenters
  def initialize(item)
    @item = item
  end

   def released
     @item.released.group_by(&date_group)
   end

   def coming_soon
     @item.coming_soon.group_by(&date_group)
   end

   def unknown
     @item.unknown.group_by(&date_group)
   end

   def date_group 
     lambda { |item| [item.release_date.try(:strftime, "%B %d, %Y"), item.time_diff_components].join()}
   end
 end
end

then in my controller

def video_games
  @presenter = Items::IndexPresenters.new(Item.video_games)
end

def movies
  @presenter = Items::IndexPresenters.new(Item.movies)
end

def tv
  @presenter = Items::IndexPresenters.new(Item.tv)
end

and my views

%h2 Movies Released
= render 'items', item: @presenter.released
%h2 Movies Coming Soon
= render 'items', item: @presenter.coming_soon
%h2 Movies Unknown
= render 'items', item: @presenter.unknown

any further suggestions would be welcomed! Thank you!

ライセンス: CC-BY-SA帰属
所属していません StackOverflow
scroll top