Is there a more succinct way to write this Ruby function?

Just for fun, I want to try to write this Ruby function more succinctly. I imagine it can be done, I'm just not knowledgeable enough with Ruby yet to know how. Any suggestions?

  def tags
return ['Cancelled'] if cancelled?
tags = []
tags << 'Filled' if filled?
tags << 'In Progress' if in_progress?
tags
end

• @Mark: The question is indeed in the frontier with codereview. But this thing is, a code review is usually broader, on longer code that may have many things to address. Here is just "what's the best way to build an array with conditional elements?". Imo that's ok for SO. – tokland Sep 15 '12 at 12:31
• @MarkThomas, I knew but I forgot. Thanks for the reminder. – Jason Swett Sep 15 '12 at 12:48

def tags
if cancelled? then ["Cancelled"]
else [*("Filled" if filled?), *("In Progress" if in_progress?)]
end
end


If you design your code so that filled? and in_progress? be false whenever cancelled? is true, then the logic will be much prettier:

def tags
[
*("Cancelled" if cancelled?),
*("Filled" if filled?),
*("In Progress" if in_progress?),
]
end

• I think this is the best answer, although what I actually ended up doing was a slight variation on your first suggestion. First line: return ['Cancelled'] if cancelled?, second line: [*("Filled" if filled?), *("In Progress" if in_progress?)] – Jason Swett Sep 15 '12 at 13:22
• The splats (*) here are superfluous and can be removed. – Andrew Marshall Sep 15 '12 at 13:25
• @AndrewMarshall You would be left with nil if you didn't have the splats. That's the whole point of this answer. – sawa Sep 15 '12 at 13:28
• @sawa Whoops I thought there was a compact in there (which I would prefer anyway). – Andrew Marshall Sep 15 '12 at 13:31
• @sawa - I have only seen the splat operator in method definitions like def foo(*args), for example. I never knew it can do this. Can you give more info about it (a link?)? Can we say - if you have conditional values in an array, put a splat before them to eliminate nil values? – Alexander Nov 13 '13 at 22:59

Functional approach:

def tags
if cancelled?
["Cancelled"]
else
[("Filled" if filled?), ("In Progress" if in_progress?)].compact
end
end


Ideas behind the snippet:

• Don't perform imperative side-effects on arrays (unless performance is a problem, not the case here).
• Use the pattern: array of "nilable" expressions + compact to build arrays with conditional elements.
• Minimize the use of inline conditional statements, they make a function harder to understand. They are ok as guards (early exit of functions), but not here. Note that I do use inline ifs, but they are expressions, not statements; more orthodox -but also more verbose- would be (filled? ? "Filled" : nil).

Not exactly the same, but here's another idea:

def tags
[ :cancelled?, :filled?, :in_progress? ].map do |m|
m.to_s if send( m )
end.compact
end

• LOL, I thought of the same thing. It can get closer in Rails (or by requiring active_support) if you add .titleize to the m.to_s. – Mark Thomas Sep 15 '12 at 13:56