0

Let us say I have this helper method that checks if a person is on above average height or below height average base on model Person's attribute height and country. person_helper.rb

module PersonsHelper
  def height_status(country, height)
    if country == "jp"
      return "above height average" if height >= 170
      return "below height average"
    end
    ... 
  end

end

Will this be a good practice or I should make a derived attribute in model for this person.rb

class Person

  def height_status
    if country == "jp"
      return "above height average" if height >= 170
      return "below height average"
    end
  end

end

The usage is mainly for view so I am wondering if helper is the right practice?

  • Maybe combine the two: have `Person#height_status` pass its data to the corresponding helper method of a `Statistics` module. – Stefan Apr 02 '19 at 11:03
  • I also thought of that. Putting the method that has the if statements in the model and just call it on the helper. But the problem is I will do all of that for just one time usage in view. Can I ask what is ur thoughts on this? –  Apr 02 '19 at 11:22
  • Consider to use concerns: https://stackoverflow.com/questions/14541823/how-to-use-concerns-in-rails-4 – iGian Apr 02 '19 at 11:46

2 Answers2

2

I would do something like this:

class Person
  AVERAGE_HEIGHTS_BY_COUNTRY = {
    jp: 170,
    us: 180,
    ...
  }

  def above_average_height?
    height > AVERAGE_HEIGHTS_BY_COUNTRY[country]
  end
end

And then use that boolean in your view:

<%= person.above_average_height? ? "very tall" : "a bit short" %>

If you wanted to use a helper to produce that explanation based on the boolean value you could. Or a better option might be to use I18n.

Jon
  • 10,678
  • 2
  • 36
  • 48
  • Hi , model Person contains the country of the person. Please check my question I think it will affect you answer greatly. Also I am not asking for the best practices on magic numbers and stuff. My main question is about the usage of helper /model. –  Apr 02 '19 at 10:55
  • Who mentioned magic numbers? I was just trying to avoid you having a 500-line if statement. I'll update for country being an attribute of person. – Jon Apr 02 '19 at 10:58
  • can you explain why using model is better than helper method ? I thought of both approaches and concluded that if I put a method exclusive for view in a model it will be a bad code but if I put it in helper my code won't look like it is object oriented. Can you tell me your opinion on this? –  Apr 02 '19 at 11:05
  • Helpers are for helping you display content. They are not where you put your business logic. – Jon Apr 02 '19 at 11:17
0

I would ask these questions:

  • Is it a person's responsibility to know their height_status for various countries? (And if it was, is returning an English localized string something the correct response?) Or
  • is it a view's concern? Something that only needs to exist because you want to show or style a string on a page?

When you think a person should know about this, then place the code into the model. If you need this information only in an HTML view, then it should be placed in a Helper or Presenter.

spickermann
  • 100,941
  • 9
  • 101
  • 131
  • I am most inclined to use Helper because it will be only used in view. But as @Jon said the helper shouldn't have logic on it like if statements. –  Apr 02 '19 at 11:20
  • It is not the person's responsibility to know their height_status, it is mainly just views concern. –  Apr 02 '19 at 11:26
  • Jon wrote that Helper should not contain business logic. I am pretty sure we meant complex rules. He wrote nothing about simple `if` statements. And I would argue that having simple conditions in helpers is perfectly fine. – spickermann Apr 02 '19 at 12:31