Is this a bad regex pattern?

Question

In a recent talk to junior JavaScript developers I mentioned the following technique for cutting down laborious if/else blocks where the OR operand is used:

if (/^(cat|dog|horse)$/.test(animal)) { ... }

rather than

if (animal == 'cat' || animal == 'dog' || animal == 'horse') { ... }

I've never had a problem with this but one person suggested it was a bad design pattern, without elaborating on why.

Answer 1

It looks to me to be "trying to be too clever" and, in doing so, you've introduced several new potential points of failures (in the regex syntax), and made the code less expressive/idiomatic. You'd also struggle if your operands were to go from being 'cat' to being dynamic or variable in some way.

Typically, in doing things like this, I only go so far as introducing an array:

if (['cat', 'dog', 'horse'].indexOf(animal) != -1) { ... }

Honestly, though, this is all laughably subjective so there is no "right" answer I can give you.

Usually at this point I'd introduce performance concerns, but you may actually have a faster solution here due to the need to scan the input only once. That would depend upon how quickly the regex itself can be parsed.