Hacker News Reader

Comment by _fat_santa | Hacker News Reader

I think these packages are just holdovers from "Before ES6". I can see it be reasonable to pull in a package like this back before the functionality was implemented in native JS. All these packages are just holdovers that are only used by:

* Legacy Projects that pulled them in way back when and never migrated to the native API's

* Inexperienced developers not knowing this is part of standard JS now.

You can't really do much about the former. For the latter though, NPM would be wise to plaster a link to MDN's docs on some of these packages.

View on HN

But "isEven"???

That's one line of code in your utils module...

In typescript? Sure. In vanilla JS? It’s more complicated. How do you handle someone inputting undefined/null/a string/a random object/NaN/etc?

For isEven, you first need isNumber. And that’s where the complexity lies.

    ({}) % 2 == 0 // false
    ([]) % 2 == 0 // true
    "" % 2 == 0 // true
    "a" % 2 == 0 // false
    "1" % 2 == 0 // false
    "2" % 2 == 0 // true
    undefined % 2 == 0 // false
    null % 2 == 0 // true

typeof x !== “number” ? false : x % 2 === 0

Somebody in your codebase is going to use !isEven(x) to test for isOdd and introduce a bug. IMO, it should crash if passed a non-number.

Actually it's the other way around. The isEven package has one dependency, which is the isOdd package.

I am literally not joking. https://www.npmjs.com/package/is-even

https://www.npmjs.com/package/is-odd-and-even

I don't know what to say.

  /**
   * is-odd-and-even
   * Github: https://github.com/fabrisdev/is-odd-and-even
   */

  import isOdd from 'is-odd'
  import isEven from 'is-even'

  /**
   * @param {number | string} i The number to check if it's odd and even
   * @returns {boolean} True if the number is odd and even, false otherwise
   */
  export default function isOddAndEven(i){
      return isOdd(i) && isEven(i)
  }

IsOdd and IsEven are so famous that it would not surprise me at all if there a bunch of joke modules using them.

Independent of the insanity that is the npm ecosystem, the API design skill of handling invalid input in a bug-resistant way is important. If somebody sent me a code review with isEven implemented as "typeof x !== “number” ? false : x % 2 === 0" I'd tell them to fix it.

How do you handle someone inputting undefined/null/a string/a random object/NaN/etc?

Your isEven function should check that it's operating on a number. Eg "const isEven = (i) => typeof i === 'number' && i % 2===0;" If it's not a number then calling isEven on it doesn't make sense. If the user wants to check if a string is even then they can convert it before making the call.

The irony of using the isEven as an example of what you should just write yourself and then providing an example that returns a truthy value for isEven(3).

No idea what you mean. ;)

Now someone is going to define const isOdd = (i) => !isEven(i); and you’ll get trouble.

A proper solution would be along the lines of:

    const isRealNumber = (n) => {
      if (typeof n !== 'number') return false;
      if (isNaN(n)) return false;
      if (n === +Inf || n === -Inf) return false;
      return true;
    };

    const isEven = (n) => {
      if (!isNumber(n)) throw new Error("Numerical error: Invalid input");
      return n % 2 === 0;
    }

    const isOdd = (n) => {
      if (!isNumber(n)) throw new Error("Numerical error: Invalid input");
      return n % 2 !== 0;
    }

This is the additional complexity created by dynamically weakly typed languages. And that’s why we get all these BS tiny npm packages.

You don't handle it. Garbage in, garbage out.

Or garbage in => throw an exception and crash the app. This stuff should be fixed as soon as possible instead of invalid data slowly spreading in every layers of the app.

Assuming you're importing it as a black box abstraction, then absolutely.

'isEven' really shouldn't be a dependency though, either through npm or internally. 'x % 2 === 0' works fine for integers and can be inlined, or if you're using it a bunch and want slightly cleaner code, you can define it as a lambda alongside the code that uses it. Then everyone can see exactly what it's doing with the full context.

The real issue is overabstraction. Even if you do everything "right" and have it as an internal dependency with all the correct error handling, it still makes the code a pain in the ass to read.

No.

If the goal of isEven is to handle user input, it is woefully misnamed.

Right, it should be "isUserInputEven" /s

* Tutorials written in the pre-ES6 world being copied and used in the ES6 world.

Reply on news.ycombinator.com