7 ms·
Yes and is-odd contains const isNumber = require('is-number'); module.exports = function isOdd(value) { const n = Math.abs(value); if (!isNumber(n
by maxbaines 5y ago
Yes and is-odd contains
const isNumber = require('is-number');
module.exports = function isOdd(value) {
const n = Math.abs(value);
if (!isNumber(n)) {
throw new TypeError('expected a number');
}
if (!Number.isInteger(n)) {
throw new Error('expected an integer');
}
if (!Number.isSafeInteger(n)) {
throw new Error('value exceeds maximum safe integer');
}
return (n % 2) === 1;
};
- pibi 5y agoI love the fact that this small code is still wrong: var isOdd = require("is-odd"); console.log(isOdd([1])); // TRUE!!
- maxbaines 5y agoFor good measure is-number contains 'use strict'; module.exports = function(num) { if (typeof num === 'number') { return num - num === 0; } if (typeof num === 'string' && num.trim() !== '') { return Number.isFinite ? Number.isFinite(+num) : isFinite(+num); } return false; };
- deleted 5y ago[deleted]
- rahkiin 5y agoFor those wondering: num-num===0 is a check for NaN.
- afavour 5y agoSmart, but isNaN already exists… https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
- Ajedi32 5y agoIt didn't at the time this library was created. I assume now that line only exists for backwards compatibility.
- jazzyjackson 5y agothis library uses the global isFinite, which actually you could just use instead of the whole function, since it does the string coercion and infinity check for you. Check out the compatibility table on MDN, Chrome Firefox and Safari version 1 all support it, so I'm pretty sure isNaN and isFinite were in the original spec.
- Ajedi32 5y agoSee the MDN section on "confusing special case behavior"[1] for why they probably didn't want to use the global version of isNaN. Number.isNaN handles those cases better but is not as widely supported. [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#confusing_special-case_behavior https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
- jazzyjackson 5y agoThanks for this, I had forgot that empty strings get coerced to 0... interesting choice.
- afavour 5y agoThe MDN compatibility data says IsNaN was in node as of 0.10.0, which was released 2013-03-11. NPM says the library was first published 2014-09-21. And the library uses isFinite which was added in the same Node version. So... not really, no. Anyway, my point wasn't so much that the code is an outrage, more that should anyone stumble across this comment thread they know that there is a much clearer and more obvious way to check for IsNaN than using x-x===0.
- 5y ago
- jazzyjackson 5y agoAlso a check for Infinity (Infinity - Infinity -> NaN) Which is weird since they use the isFinite built-in right after that, but I have no idea why they throw in all the string checks and trims and + for type coercion, and then use the global isFinite function which does all that internally. In fact the global function isFinite does everything this package does (something I just learned tho checking this code out is that there is a global isNaN and isFinite which coerce strings into numbers automatically, and Number.isNaN and Number.isFinite which function similarly but without coercing the string first, so I guess this package exists as a learning aide of all the different ways to ask if something is a number) EDIT: I now know why they check if a string input is not an empty string, because empty strings get coerced to 0, so isFinite("") returns true.
- rezonant 5y ago> EDIT: I now know why they check if a string input is not an empty string, because empty strings get coerced to 0, so isFinite("") returns true. Yes, when coerced with +: Number.isFinite(+'') // true Number.isFinite('') // false
- jazzyjackson 5y agoNumber.isFinite('') and Number.isNaN('') returns false, isFinite('') and isNaN('') returns true, the global versions of isFinite and isNaN first coerce their input to number, no '+' necessary. Hence the heading "Confusing Special Case Behavior" in the mdn docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#confusing_special-case_behavior https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
- rezonant 5y agoOh yes the global versions.
- hajile 5y agoI'd add that `num !== num` would be faster while `Number.isNaN(num)` would be faster and more readable.
- deleted 5y ago[deleted]
- deleted 5y ago[deleted]
- Ajedi32 5y agoSo in conclusion... maybe checking whether a number is even in JS isn't as trivial as everyone thought, and projects reusing that code was a good thing? Or are are all those extra checks unnecessary now that TypeScript exists and new functions like Number.isInteger have widespread browser support?
- hmrr 5y agoI think the assertion (sorry for the pun) is that JavaScript is a completely unholy creation and should be chucked off the edge of the disc. I can’t imagine trying to get some of our fintech business logic in there without arming a nuclear foot gun or ten. I would never sleep again without medication.
- mannerheim 5y agoIf so, the implementation of isEven isn't even right because it assumes anything that isn't an odd number is an even number.
- Ajedi32 5y agoNo, the code posted in this thread clearly indicates it will raise an exception in such cases.
- mannerheim 5y agoAh, that's right, never mind.
- runes 5y agoTypeScript can't help at all, it vanishes before the code runs.
- tinco 5y agoThat horrible realisation that because of its dependency on is-number, and all of its checks and pretty error messages I can imagine there's actually some valid use cases for this package. You can plug this right into a user input validation system and it would work perfect for production use cases in a way that num % 2 === 0 just wouldn't. Almost other use case for it though.. horrible. This package would be much better if it was in some sort of namespace that indicated it's for user input validation only.