Discussion on: Which is more readable?

[Scott Tadman]

I'm a big proponent of trying to avoid "surprise if conditions", in other words a line that might, for reasons of not being shown in full in your code, not actually run like you think it does, but instead (surprise!) only run under certain conditions.

So things like this are fine:

return if x
return false unless y
next if z

Since you can see at a glance exactly what's going on there even if the conditions are fairly long.

That being said, a refactoring that fixes this up probably looks like this:

json_data = {
  user: add_user_data(mentioner)
}

case (mentionable_type)
when 'Comment'
  json_data[:comment] = add_comment_data(mentionable)
end

This is the "define and conditionally augment" approach to accumulating data in an object. The use of the case here makes it clear where to add any other cases should they arise.

[Andy Zhao (he/him)]

Not a bad idea, thanks for sharing! Personally never liked cases but this might be a good use for them.