Is it better to use || or include? when checking a variable against multiple values?

Question

Which way of writing this condition is better?

1)

(self.expense_gl_dist_code.dist_type == GlDistCode::PERCENTAGE || self.expense_gl_dist_code.dist_type == GlDistCode::MIXED)

2)

["GlDistCode::PERCENTAGE","GlDistCode::MIXED"].include?(self.expense_gl_dist_code.dist_type)

Answer 1

The first way is much clearer and therefore to be preferred to the slightly obfuscated second option.

Answer 2

Just split 1st into two lines.

Answer 3

I find the second clearer for two reasons:

1) In the second version the elements being checked for are all next to each other, separated by commas. In the first version there's always self.expense_gl_dist_code.dist_type ==, so it's less easy to scan them all at once.

2) In the second version it's immediately obvious that all elements are checked for the same condition, while in the first version it could say something like

dist_type == GlDistCode::PERCENTAGE || dist_type == GlDistCode::MIXED || dist_type != GlDistCode::WHATEVER

and you might not notice right away.

Answer 4

If you are just comparing two elements, I'd say either is fine.

I'm more inclined to the second version, because you can then include all the elements you want to validate against into a single variable, and name it. For example

ALLOWED_TYPES = [GldDistCode::PERCENTAGE, GlDistCode::MIXED]

then

if ALLOWED_TYPES.include?(dist_type)

is more legible IMHO.

BTW, you're using strings ("GldDistCode::PERCENTAGE") instead of the actual value which you intended.