Please anchor your regexes & stop driving me to distraction

Here’s my new maxim: Code what you mean. Check this snippet from the JS side of a vastly popular platform:


If you mean, as I’m 100% sure the author did…

if document.readyState does not case-sensitively equal the string "loaded" nor the string "complete"

… that’s great, but you’ve gotta implement that logic!

In contrast, the regex above actually checks…

if document.readyState does not case-sensitively contain the string "loaded" nor the string "complete"

… which is not the same.

“But it works, and you knew what I meant!” you might respond. Sorry, but that doesn’t matter, because you didn’t code what you meant.

Yes, “unloaded” and “incomplete” and “NP-complete” happen to not (currently) be valid readyState values, so the code can’t break stuff in (current) practice. But a person reading your code shouldn’t need to know that. If they tried to refactor while preserving the meaning of your code, they’d end up perpetuating the not contains logic unnecessarily. And eventually it would become a “don’t touch” part of the codebase, though it was wrong from the start.

Coding what you meant:

if(document.readyState && !/^(loaded|complete)$/.test(document.readyState))

Use the alternation operator | as in the original, but anchor the 2 values so they must comprise the entire string (^ at the start, $ at the end).

IMO, using RegExp#test for this is totally fine. It’s compact and readable and doesn’t require browser support for Array#includes (the ES6 test against collections).

I’m not a regex expert, but one thing’s for sure: a regex that doesn’t do what you meant, and only what you meant, is bad code.