CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3
danielwilber
danielwilber

Reputation: 65

Cleaning up duplicate controller code

The Dillema:

I am in a situation where I need to show a User's activity across multiple views and controllers. The code is starting to smell a little bit, so I'd really appreciate some guidance in cleaning it up.

I have a User controller has a "show" (/user_id/) view that renders recent activity in the sidebar, as well as a "activity" (user_id/activity) view that renders all of the user's activity. I also have a "Pages" controller that has a "dashboard" view that renders a user's activity as well.

The Code:

users_controller.rb looks like:

@activities = []
@activities += @user.skills.map { |skill| Activity.new("skill", skill, skill.created_at) }
@activities += @user.unions.map { |union| Activity.new("union", union, union.created_at) }
@activities += @user.companies.map { |company| Activity.new("company", company, company.created_at) }
@activities += @user.clients.map { |client| Activity.new("client", client, client.created_at) }
@activities += @user.schools.map { |school| Activity.new("school", school, school.created_at) }
@activities += @following_list.map { |following| Activity.new("following", following, following.created_at) }
@activities += @recommended.map { |recommend| Activity.new("recommend", recommend, recommend.created_at) }
@activities += @user.statuses.all(:limit => 5, :order => "created_at DESC").map { |status| Activity.new("status", status, status.created_at) }

sorted_activities = @activities.sort_by(&:date).reverse
@activities = sorted_activities[0..(4)]

pages_controller.rb looks like:

@your_activity = []
@your_activity += current_user.skills.map { |skill| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","skill", skill, skill.created_at) }
@your_activity += current_user.unions.map { |union| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","union", union, union.created_at) }
@your_activity += current_user.companies.map { |company| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","company", company, company.created_at) }
@your_activity += current_user.clients.map { |client| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","client", client, client.created_at) }
@your_activity += current_user.schools.map { |school| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","school", school, school.created_at) }
@your_activity += current_user.statuses.map { |status| ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}","status", status, status.created_at) }

sorted_activities = @your_activity.sort_by(&:date).reverse
@your_activity = sorted_activities[0..(35)]

Upvotes: 2

Views: 323

Answers (1)

agmcleod
agmcleod

Reputation: 13621

There's a few ways you can do it really. You could offload it to the model, you could create a module in the lib directory and use that to build the activity arrays. One major thing too is to be sure to use a helper for displaying it on the page, and load that either into the views or a partial, depending on your needs.

You could also reduce the duplication a bit with some metaprogramming. You may need to adjust depending on your requirements. You'll need to set it up for the first set of code as well

["skill", "union", "company", "client", "school", "status"].each do |type|
  plural = ActiveSupport::Inflector.pluralize(type)
  @your_activity += current_user.send(plural.to_sym).map do |object|
    ActivityDashboard.new("#{current_user.first_name} #{current_user.last_name}","#{current_user.id}","#{current_user.avatar}",type, object, object.created_at)
  end
end

Gist of my working example: https://gist.github.com/3669117

Upvotes: 1

Related Questions