Preacher wrote:
> I have some helper code that I let get out of hand. I have a number of
> show actions that are routed by 'controller/:permalink' and three show
> actions that are not. The blog controller's show action gets its page
> title, meta description, and meta keywords by parsing some json
> content from another site over which I have limited control. The
> messages and site_images models have no permalink column. This is what
> I've written to get all this to work right, but it looks really ugly
> (and potentially brittle) to me. I'm open to suggestions on how to DRY
> this up and make it solid. (I'm sticking with 2.3.8 for a while before
> I tackle 3, so if you could limit your suggestions to Rails 2....)
>
>
> def page
> if controller_name == 'blog' and action_name == 'show'
> page = StaticPage.find(:first, :conditions => { :url =>
> 'rescue' })
> elsif controller_name == 'messages' and action_name == 'show'
> page = StaticPage.find(:first, :conditions => { :url =>
> 'rescue' })
> elsif controller_name == 'site_images' and action_name == 'show'
> page = StaticPage.find(:first, :conditions => { :url =>
> 'rescue' })
> elsif action_name == 'show'
> page = @model_name.find_by_permalink(params[:permalink])
> else
> page = StaticPage.find(:first, :conditions => { :url =>
> "#{controller_name}/#{action_name}" }) ||
> StaticPage.find(:first, :conditions => { :url => 'rescue' })
> end
> end
Yuck! That should all be handled in your routes file.
>
> def page_title
> if controller_name == 'blog' and action_name == 'show'
> title = @post.first['title']
> else
> title = page.title || 'domain.com'
> end
> %(<title>#{title}</title>)
> end
>
I think you've tried to DRY up your helpers past the point of
sensibility, and wound up with something so generic that it doesn't make
sense.
Also: @post.first ? @post is an array of multiple posts? If so, then
its name should be @posts.
> def meta(name, content)
> %(<meta name="#{name}" content="#{content}" /> )
> end
That looks OK -- if you need a helper for this. I'm not sure why you
would.
Note also that the /> is only valid if you're using XHTML (which you
shouldn't be) or HTML 5. If you're using HTML 4, get rid of the slash
and install the html_output plugin.
>
> def meta_description
> if controller_name == 'blog' and action_name == 'show'
> parse_meta_description(@post.first['content'])
> else
> page.meta_description
> end
> end
>
> def meta_keywords
> if controller_name == 'blog' and action_name == 'show'
> parse_meta_keywords(@post.first['content'])
> else
> page.meta_keywords
> end
> end
Why not just have these as model methods?
>
>
> Thanks in advance
Best,
--
Marnen Laibow-Koser
http://www.marnen.org
[email protected]
--
Posted via http://www.ruby-forum.com/.
--
You received this message because you are subscribed to the Google Groups "Ruby
on Rails: Talk" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.
