Kleber S.
Kleber S.

Reputation: 8260

How can I refactor this code for Ruby?

I created the following helper method and it is working fine but I want to improve the way it is being called.

def display_if_present(attribute, i18key, hours=nil)
  unless attribute.blank?
    content_tag :li do

      if hours.present?
        concat content_tag :h2, get_translation_by_model_attr('achievment', i18key) + ' ('+t(:hours)+'): '
      else
        concat content_tag :h2, get_translation_by_model_attr('achievment', i18key)+': '
      end

      if i18key.include? 'date'
        concat content_tag :p, attribute.strftime('%m/%Y')
      elsif hours.present?
        concat content_tag :p, h(attribute) + "(" + t(:hours) + ")"
      else
        concat content_tag :p, h(attribute) 
      end
    end
  end
end

And on the view side I have the following:

<%= display_if_present(academic_achievment.institution,'institution') %>

<%= display_if_present(academic_achievment.ativs_description,'ativs_description') %>

<%= display_if_present(academic_achievment.date_start,'date_start') %>

<%= display_if_present(academic_achievment.date_finished,'date_finished') %>

<%= display_if_present(academic_achievment.load, 'load', :hours) %>

<%= display_if_present(academic_achievment.tags, 'tags') %>

which I want to do some refactor. So I tried:

<% elements = %w(institution ativs_description date_start date_finished load tags) %>

<% elements.collect! do |item| %>
  <% params = (item == "load") ? "item, :hours" : "item" %>
  <%= display_if_present("academic_achievment.#{item}".constantize, params.constantize) %>
<% end %>

the above block of code returned the error:

wrong constant name academic_achievment.institution
Extracted source (around line #15):

12: 
13:         <% elements.collect! do |item| %>
14:           <% params = (item == "load") ? "item, :hours" : "item" %>
15:           <%= display_if_present("academic_achievment.#{item}".constantize, params.constantize) %>
16:         <% end %>
17:  
18:       </ul>

I appreciate some help for a better piece of code.

Upvotes: 0

Views: 79

Answers (1)

Mischa
Mischa

Reputation: 43318

I recommend using the send method like this:

<% %w(institution ativs_description date_start date_finished load tags).each do |item| %>
  <%= display_if_present(academic_achievment.send(item), item, item == 'load' ? :hours : nil) %>
<% end %>

Upvotes: 1

Related Questions