6
\$\begingroup\$

Currently, when I'm wanting to print a value for my class I do the following...

<%= @person.team.name if @person.team.present? %>

This seems really redundant to me. I've also have done...

<%= @person.team.name_display %>

where I've created a function for each attribute to kind of hide the first case. It seems a little much though. Is there a more preferred way to do it such as...

<%= @person.team.name || "" %>
\$\endgroup\$
6
\$\begingroup\$

You are right, you code is too verbose. This is a pretty common pattern and you have some alternatives. For example, active_support has the abstraction Object#try:

<%= @person.team.try(:name) %>

Another alternative is the Object#maybe proxy: https://github.com/bhb/maybe

<%= @person.team.maybe.name %>
\$\endgroup\$
4
\$\begingroup\$

You need to create a delegate method on your Person class:

class Person
  def team_name
    team.name unless team.nil?
  end
end

Then it's just a simple:

<%= @person.team_name %>

If you have a lot of these, consider using the Delegate module:

class Person
  delegate :name, :ranking, :jersey, :grounds,
             :to => :team, :allow_nil => true, :prefix => true
  # Person now responds to #team_name, #team_ranking, #team_jersey, #team_grounds as above
end

The reason this approach works better has to do with the Principle of Least Knowledge:

The Law of Demeter (LoD) or principle of least knowledge is a design guideline for developing software, particularly object-oriented programs. In its general form, the LoD is a specific case of loose coupling. The guideline was proposed at Northeastern University towards the end of 1987, and can be succinctly summarized in each of the following ways:

  • Each unit should have only limited knowledge about other units: only units "closely" related to the current unit.
  • Each unit should only talk to its friends; don't talk to strangers.
  • Only talk to your immediate friends.

(Emphasis, mine)

The last bullet point is the key. Your view should only be calling methods on @person. In order to get the team name, your view should not need to test for the existence of @person.team. That requires your view to have too much knowledge of the @person object, and instead it is preferable to create a delegate method in Person that checks for team.nil?.

\$\endgroup\$
  • \$\begingroup\$ @ymbirtt: Good catch on the Delegate module. I forgot about that one. \$\endgroup\$ – Greg Burghardt Nov 2 '15 at 21:39
1
\$\begingroup\$

You can also use the safe navigation operator like so: <%= @person.team&.name %>

http://mitrev.net/ruby/2015/11/13/the-operator-in-ruby/

\$\endgroup\$
0
\$\begingroup\$

I believe the idiomatic Rails approach is to use Object#presence:

<%= @person.team.name.presence || '' %>

This checks for false, empty, whitespace and nil.

\$\endgroup\$
  • \$\begingroup\$ But what if team is the nil object, not name? I can't test that case right this second but I don't believe the above will work in that instance. \$\endgroup\$ – moopasta Nov 2 '15 at 16:03
  • \$\begingroup\$ Then you would need to use try's: <%= @person.try(:team).try(:name).presence || '' %> \$\endgroup\$ – Dan Kohn Nov 2 '15 at 16:15
  • 1
    \$\begingroup\$ how is that better than <%= @person.team.try(:name) %>? I do not mean that to come off as a harsh response either, just curious if it's truly a better way. Thanks! \$\endgroup\$ – moopasta Nov 2 '15 at 16:17
  • \$\begingroup\$ The advantage of presence is how it deals with an empty string or array value. It would particularly make sense if you do: @person.try(:team).try(:name).presence || 'Default Value' and want Default Value to show if name returns an empty string. \$\endgroup\$ – Dan Kohn Nov 2 '15 at 17:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.