need some opinions

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

  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

  def meta(name, content)
    %(<meta name="#{name}" content="#{content}" /> )
  end

  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

Thanks in advance

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,

Earlier this year, I started going for HTML 5.

I don't know why, but I have to specify the first @post from the json
content I'm parsing even though my query only returns one result.

I hadn't thought of defining the page in the models....hmm time to
create a new branch. Thanks for the idea.

[Please quote when replying -- it makes the discussion easier to
follow.]

Preacher wrote:
[...]

I don't know why, but I have to specify the first @post from the json
content I'm parsing even though my query only returns one result.

Then you should find out why. Is the JSON returning an array,
perchance?

I hadn't thought of defining the page in the models....

What? I have no idea what you're talking about here, and you didn't
quote what you were referring to.

hmm time to
create a new branch. Thanks for the idea.

I'm not sure I gave you that idea...

Best,