CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3refactoringdry
chell
chell

Reputation: 7866

Want to DRY this code but have trouble

I have two methods that do similar things. I am a noob and want to know how I might be able to make these combine into one method:

#test if the current selected language is the one that was clicked in the menu
def link_to_without_class_unless_current_language(language)
  if language_selected?(language)
    content_tag(:li, content_tag(:span, content_tag(:em, language)), :class => "current")
  else
    content_tag :li, link_to(content_tag(:span, language), :locale => language.prefix)
  end
end


#tests if the current page is the same as that for the link
def link_to_without_class_unless_current(name, options)
  if current_page?(options[:url])
    content_tag(:li, content_tag(:span, content_tag(:em, name)), 
                :class => options[:class] ||= "current")
  else
    content_tag :li, link_to(content_tag(:span, name), options[:url])
  end
end

In the View:

<%= link_to_without_class_unless_current_language 'English'  %> |
<%= link_to_without_class_unless_current_language 'Français' %>

<%= link_to_without_class_unless_current t('application.menu_links.home'), 
                                         { :url => root_url } %>

Upvotes: 1

Views: 88

Answers (1)

Andrew Lank
Andrew Lank

Reputation: 1617

Might I suggest that you simply pass a collection of options instead, just like your second parameter in your second method, where you verify if options[:url], you could verify if options[:language] is set, then perform the code in your first method's code, else if option[:url] is set perform the second method's code, you might as well offer the option :name as well.

so your 3 calls would look like this

<%= link_to_without_class_unless_current :language => 'English'  %> |
<%= link_to_without_class_unless_current :language => 'Français' %>

<%= link_to_without_class_unless_current :name => t('application.menu_links.home'), 
                                         :url => root_url  %>

Upvotes: 1

Related Questions