C8c549e9bb39135192db53d238e19f3e

This works but it's kind of ugly... Need help to cut down the lines in a good and smart way. Suggestions?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
module DashboardHelper
  def activity_message_for(object)
    case object.class.name
      when 'Job'
        job_image_tag = image_tag("snippet.png", :class => "icon")
        if object.created_at == object.updated_at
          "<td>#{job_image_tag} The job #{link_to object.title, job_path(object)} 
          was added.</td><td> #{object.updated_at.to_formatted_s(:short)}</td>"
        elsif object.updated_at > object.created_at
          "<td>#{job_image_tag} The job #{link_to object.title, job_path(object)} 
          was updated.</td><td> #{object.updated_at.to_formatted_s(:short)}</td>"
        end
      when 'Candidate'
        candidate_image_tag = image_tag("candidate.png", :class => "icon")
        if object.created_at == object.updated_at
          "<td>#{candidate_image_tag} The Candidate #{link_to object.name, job_candidate_path(object.job_id, object)} 
          was added to #{link_to object.job.title,job_path(object.job)}.</td><td> #{object.created_at.to_formatted_s(:short)}</td>"
        elsif object.updated_at > object.created_at
          "<td>#{candidate_image_tag} The Candidate #{link_to object.name, job_candidate_path(object.job_id, object)} 
          that belongs to job #{link_to object.job.title,job_path(object.job)} was updated.</td><td> #{object.updated_at.to_formatted_s(:short)}</td>"
        end
      when 'Comment'
        comment_image_tag = image_tag("comment.png", :class => "icon")
        if object.created_at == object.updated_at
          "<td>#{comment_image_tag} A comment was added to candidate 
          #{link_to object.candidate.name, job_candidate_path(object.candidate.job_id, object.candidate_id)} by
          #{get_pretty_user_from_object(object)} </td><td> #{object.created_at.to_formatted_s(:short)}</td>"
        end
    end
  end
end

Refactorings

No refactoring yet !

D16d53391068ff0830269149b060789d

Jason Dew

July 21, 2008, July 21, 2008 22:59, permalink

3 ratings. Login to rate!

I would consider moving the cases into their respective helpers and calling them that way:

So you would define a method in JobHelper, CandidateHelper, and CommentHelper called activity_message. Hope this helps,

1
2
3
4
5
6
7
module DashboardHelper

  def activity_message_for object
    "#{object.class.name.classify}Helper".constantize.send(:activity_message, object)
  end

end
880cbab435f00197613c9cc2065b4f5a

danielharan

July 21, 2008, July 21, 2008 23:01, permalink

No rating. Login to rate!

HTML inside helpers is ugly, get that into partials and it gets way easier to deal with:

1
2
3
4
5
module DashboardHelper
  def activity_message_for(object)
    render :partial => "#{object.class.name.downcase}/_#{object.class.name.downcase}_activity"
  end
end
880cbab435f00197613c9cc2065b4f5a

danielharan

July 22, 2008, July 22, 2008 12:54, permalink

No rating. Login to rate!

Argh - hopefully now I'm awake:

1
2
3
4
5
module DashboardHelper
  def activity_message_for(object)
    render :partial => "#{object.class.name.downcase.pluralize}/activity"
  end
end
C8c549e9bb39135192db53d238e19f3e

Kryckan

July 22, 2008, July 22, 2008 18:38, permalink

No rating. Login to rate!

Thanks! Will try it out now!

Your refactoring





Format Copy from initial code

or Cancel