It’s kind of insane how bad this whole is-number thing is. It’s designed to tell you if a string is numeric, but I would argue if you’re ever using that you have a fundamental design problem. I hate dynamic typing as much as anyone else, but if forced to use it I would at least try to have some resemblance of sanity by just normalizing it to an actual number first.
Just fucking do this…
consttoRegexRange = (minStr, maxStr, options) => {
const min = parseInt(minStr, 10);
const max = parseInt(maxStr, 10);
if (isNaN(min) || isNaN(max)) throwError("bad input or whatever");
// ...
Because of the insanity of keeping them strings and only attempting to validate them (poorly) up front you open yourself up to a suite of bugs. For example, it took me all of 5 minutes to find this bug:
toRegexRange('+1', '+2')
// returns "(?:+1|+2)" which is not valid regexp
The problem is the underlying API. parseInt(“550e8400-e29b-41d4-a716-446655440000”, 10) (this is a UUID) returns 550. If you’re expecting that input to not parse as a number, then JavaScript fails you. To some degree there is a need for things to provide common standards. If your team all understands how parseInt works and agrees that those strings should be numbers and continues to design for that, you’re golden.
Yeah good point. I suppose the problem is this function that operates on numbers allows numeric strings to be passed in in the first place. The only place where I would really expect numeric strings to exist is captured directly from user input which is where the parsing into a numeric data type should happen, not randomly in a library function.
It’s kind of insane how bad this whole
is-numberthing is. It’s designed to tell you if a string is numeric, but I would argue if you’re ever using that you have a fundamental design problem. I hate dynamic typing as much as anyone else, but if forced to use it I would at least try to have some resemblance of sanity by just normalizing it to an actual number first.Just fucking do this…
const toRegexRange = (minStr, maxStr, options) => { const min = parseInt(minStr, 10); const max = parseInt(maxStr, 10); if (isNaN(min) || isNaN(max)) throw Error("bad input or whatever"); // ...Because of the insanity of keeping them strings and only attempting to validate them (poorly) up front you open yourself up to a suite of bugs. For example, it took me all of 5 minutes to find this bug:
toRegexRange('+1', '+2') // returns "(?:+1|+2)" which is not valid regexpThe problem is the underlying API.
parseInt(“550e8400-e29b-41d4-a716-446655440000”, 10)(this is a UUID) returns550. If you’re expecting that input to not parse as a number, then JavaScript fails you. To some degree there is a need for things to provide common standards. If your team all understands howparseIntworks and agrees that those strings should be numbers and continues to design for that, you’re golden.Yeah good point. I suppose the problem is this function that operates on numbers allows numeric strings to be passed in in the first place. The only place where I would really expect numeric strings to exist is captured directly from user input which is where the parsing into a numeric data type should happen, not randomly in a library function.