Clever Geek Handbook

Is this a bad regex pattern?

In a recent conversation with junior JavaScript developers, I mentioned the following method for shortening time-consuming if / else blocks where an OR operand is used:

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

but not

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

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

+3


source to share


4 answers


I feel like I'm trying to be too smart, and in doing so, you've introduced several new potential points of failure (in the regex syntax) and made the code less expressive / idiomatic. You will also struggle if your operands were to go from 'cat'

being dynamic or variable in some way.

Usually, when doing things like this, I just got as far as introducing an array:

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


To be honest, this is all ridiculously subjective, so there is no "correct" answer I can give you.

Usually at this point I would present performance issues, but in reality you may have a faster solution due to the need to scan the input only once. It will depend on how quickly the regex can be parsed.

+4


source


Regular way of expression, no doubt, I would rather thank in this case for the long list of comparisons. Performance also I think the regex will not significantly inferior to multiple comparisons (imagine a 30-40 comparison case for your animal).



Regex also gives you other benefits, such as checking for a match, or comparing against a border word (for this case where the input is part of some text), whereas comparing strings would require different code.

+2


source


Building a regex is more costly than doing multiple string matches, but if it makes the code clearer and there is no performance impact (through profiling, of course!), Then I think it's good.

+1


source


This opinion is based, but some points for improvement:

The code should be readable, not short.

Regex is more error prone, for example: if forgotten $

this will matchhorses

Without docs, someone reading the code refactoring code can't be sick of the goal:

  • Should animals be distinguished? ^cat|dog|horse$

  • Should animals be distinguished? ^cat|dog|horse

Adding multiple options can be problematic, for example:

if (animal == 'cat' || animal == 'dog' || animal == someUserInputtedAnimal)

Decision:

For many if statements, use a switch or a "true" switch:

switch (animal)
{
    case 'cat':
        break;
    case 'dog':
        break;
    case 'horse':
        break;
}

switch (true)
{
    case animal == 'cat':
        break;
    case animal == 'dog':
        break;
    case animal == 'horse':
        break;
    case someRandomAnimal == ressurected:

        break;
}
0


source






More articles:


All Articles