# Need some opinions

**URL:** <https://www.ruby-forum.com/t/need-some-opinions/193536>\
**Category:** Rails\
**Created:** [August 19, 2010, 12:13pm UTC](https://www.ruby-forum.com/t/need-some-opinions/193536 "2010-08-19T12:13:50Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![Preacher](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/p/46a35a/32.png) [@Preacher](https://www.ruby-forum.com/u/Preacher)\
**Post date:** [August 19, 2010, 12:13pm UTC](https://www.ruby-forum.com/t/need-some-opinions/193536/1 "2010-08-19T12:13:50Z")

</div>

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](http://domain.com)’  
end  
%(#{title})  
end

def meta(name, 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

---

<div class="post-metadata">

**Author:** ![Preacher](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/p/46a35a/32.png) [@Preacher](https://www.ruby-forum.com/u/Preacher)\
**Post date:** [August 19, 2010, 5:48pm UTC](https://www.ruby-forum.com/t/need-some-opinions/193536/2 "2010-08-19T17:48:10Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![Preacher](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/p/46a35a/32.png) [@Preacher](https://www.ruby-forum.com/u/Preacher)\
**Post date:** [August 19, 2010, 4:26pm UTC](https://www.ruby-forum.com/t/need-some-opinions/193536/3 "2010-08-19T16:26:02Z")

</div>

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](http://domain.com)’  
> end  
> %(#{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)  
> %( )  
> 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](http://www.marnen.org)  
removed\_email\_address@domain.invalid

---

<div class="post-metadata">

**Author:** ![Preacher](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/p/46a35a/32.png) [@Preacher](https://www.ruby-forum.com/u/Preacher)\
**Post date:** [August 19, 2010, 6:51pm UTC](https://www.ruby-forum.com/t/need-some-opinions/193536/4 "2010-08-19T18:51:56Z")

</div>

[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,

Marnen Laibow-Koser  
[http://www.marnen.org](http://www.marnen.org)  
removed\_email\_address@domain.invalid