0
\$\begingroup\$

I have following HTML class collector. Is there a better way than []tap for this?

def tr_classes(user)
  classes = [].tap do |c|
    c << "error" if user.company.nil?
    c << "disabled" if user.disabled?
  end
  if classes.any?
    " class=\"#{classes.join(" ")}\""
  end
end

<tr<%= tr_classes(user) %>>
  <td><%= user.name %></td>
</tr>
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Notes:

  • This tap usage is not uncommon to see but IMHO it's very, very dubious. You can use a functional approach (see code below).
  • I wouldn't return the string along with the attribute, just the array of classes (which Rails3 helpers understand).

I'd write:

def tr_classes(user)
  [
    ("error" if !user.company),
    ("disabled" if user.disabled?),
  ].compact.presence
end

Tables are usually easier to build from helpers. You now would use the function that way:

content_tag(:tr, :class => tr_classes(user)) do
  content_tag(:td, user.name)
end
\$\endgroup\$
1
  • \$\begingroup\$ Glad somebody suggested content_tag; I find opening a <%= %> to mess with the individual attributes of an opening HTML tag to be particularly awful. \$\endgroup\$
    – user229044
    Dec 29, 2012 at 12:37
1
\$\begingroup\$

What do you mean a "better way" than tap? Why are you using tap at all? Initialize an array. Conditionally append items to it. This pattern is as old as time, and there is no reason to shoe-horn in a Ruby-esque solution.

classes = [].tap do |c|
  c << "error" if user.company.nil?
  c << "disabled" if user.disabled?
end

vs

classes = []
classes << "error" if user.company.nil?
classes << "disabled" if user.disabled?

You've added so much complexity, both functionally and visually, for literally no gain. You've produced more and uglier code.

\$\endgroup\$
1
  • \$\begingroup\$ I also dislike this use of tap, but I guess that the OP is using it because it's not uncommon to see. The only good thing about this pattern is that side-effects are confined into a block, that maybe useful with other objects, with an array it's completely overkill. \$\endgroup\$
    – tokland
    Jan 2, 2013 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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