jonschlinkert / is-primitive

Is the typeof value a javascript primitive?
MIT License
37 stars 9 forks source link

Removed switch case #4

Closed ekologic closed 6 years ago

ekologic commented 6 years ago

Using switch cases is not a good practice, in Javascript we can have this beautiful solution to avoid the switch case, it is more readable and extensible

jonschlinkert commented 6 years ago

Using switch cases is not a good practice

Why? I've seen this fallacy repeated numerous times, yet switch statements are often much faster in benchmarks.

edit: It would be helpful if you could link to some documentation or any kind of proof that switch-cases are sub-optimal, or at least describes specific situations in which switch-cases are sub-optimal.

jonschlinkert commented 6 years ago

lol i requested a review from myself. I just wanted to see if it would actually work. Fun.

jonschlinkert commented 6 years ago

To run the following code, add it to a file (like bench.js), then do $ npm i benchmark && node bench,

const Benchmark = require('benchmark');
const suite = new Benchmark.Suite();

const types1 = { boolean: true, number: true, string: true, symbol: true, undefined: true };
const types2 = ['boolean', 'number', 'string', 'symbol', 'undefined'];
const values = [
  'foo',
  1,
  function() {},
  {},
  new Date(),
  /foo/,
  Symbol('foo'),
  null,
  undefined,
  void 0,
  true,
  false,
  NaN,
  Infinity
];

suite
  .add('switch', () => values.map(v => isPrimitiveSwitch(v)))
  .add('object', () => values.map(v => isPrimitiveObjectLookup(v)))
  .add('object - in', () => values.map(v => isPrimitiveObjectIn(v)))
  .add('object - own', () => values.map(v => isPrimitiveObjectOwn(v)))
  .add('array', () => values.map(v => isPrimitiveArray(v)))
  .add('equals', () => values.map(v => isPrimitive(v)))

  .on('cycle', event => console.log(String(event.target)))
  .on('complete', function() {
    console.log('Fastest is ' + this.filter('fastest').map('name'));
  })
  .run({ 'async': true });

function isPrimitive(val) {
  if (val === null) {
    return true;
  }
  const type = typeof val;
  return type === 'boolean'
    || type === 'number'
    || type === 'string'
    || type === 'symbol'
    || type === 'undefined';
}

function isPrimitiveArray(val) {
  return val === null || types2.indexOf(typeof val) > -1;
}

function isPrimitiveObjectOwn(val) {
  return val === null || types1.hasOwnProperty(typeof val);
}

function isPrimitiveObjectLookup(val) {
  return val === null || !!types1[typeof val];
}

function isPrimitiveObjectIn(val) {
  return val === null || (typeof val) in types1;
}

function isPrimitiveSwitch(val) {
  switch (typeof val) {
    case 'boolean':
    case 'number':
    case 'string':
    case 'symbol':
    case 'undefined':
      return true;
    default: {
      return val === null;
    }
  }
}

// switch x 3,104,738 ops/sec ±0.61% (90 runs sampled)
// object x 1,947,977 ops/sec ±0.55% (92 runs sampled)
// object - in x 2,160,225 ops/sec ±0.63% (89 runs sampled)
// object - own x 2,376,049 ops/sec ±0.59% (88 runs sampled)
// array x 1,441,834 ops/sec ±0.54% (84 runs sampled)
// equals x 2,983,937 ops/sec ±0.59% (87 runs sampled)
// Fastest is switch

also, fwiw, IMHO the switch statement is far more readable and elegant than the code you're proposing. But my preference for aesthetics doesn't really matter, the performance is what's important.

rodenmonte commented 6 years ago

Here's another solution that is slightly faster than switch:

function isPrimitiveIf(val) {
  if (val === null
    || typeof val === 'boolean'
    || typeof val === 'number'
    || typeof val === 'string'
    || typeof val === 'symbol'
    || typeof val === 'undefined') {
    return true
  }
  return false
}

Computing typeof val 5 times turns out to be less expensive than assigning the type to a variable. The difference is small though (output of node bench.js w/ a test case added):

switch x 2,405,657 ops/sec ±0.91% (88 runs sampled)
if x 2,512,637 ops/sec ±0.80% (84 runs sampled)
object x 1,507,709 ops/sec ±0.78% (88 runs sampled)
object - in x 1,710,923 ops/sec ±0.90% (86 runs sampled)
object - own x 1,788,273 ops/sec ±1.04% (84 runs sampled)
array x 1,166,770 ops/sec ±0.82% (86 runs sampled)
equals x 2,268,414 ops/sec ±0.67% (85 runs sampled)
Fastest is if

I ran the test a few times on my machine and if was always faster, although it might be different on different machines. This definitely qualifies as a micro-optimization, and is substantially less elegant than the switch-case imo. If you think that this is an improvement, I'll submit a pull to change bench.js (include isPrimitiveIf) and index.js.