30-seconds-of-code: isEmpty will be invalid for Set and Map

[fatedeity]

Bug description

Object.keys is invalid for Set and Map, If a Set object add a value, then use Object.keys will get a empty array, the same to Map.

[haoliangwu]

maybe should check wheather the val is iterable at first, something like:

isEmpty = val => val == null || !(typeof val[Symbol.iterator] === 'function' ? [...val] : (Object.keys(val) || val)).length

result screenshot:

but not sure if it is 100% correct.

[skatcat31]

For map or set you can also check the .size property. However there is a problem with using ors at that point:

const isEmpty = val => val == null || (val.hasOwnProperty('length') && !val.length) || !val.size || !Object.keys(val).length;

isEmpty(new Set()) // true... uh oh
isEmpty([1]) // true... oh my

This fails on valid maps and sets. At this point isEmpty() would need to be a longer function doing some duck checking to make sure it makes easy sense and is maintainable:

const isEmpty = val => {
  if (val == null) return true;
  if ('length' in val) return !val.length;
  if ('size' in val) return !val.size;
  return !Object.keys(val).length;
}

And yes you could use the iterator check, but the less time complexity the better. You could also check for the first case to make sure .length or .size are explicitly 0 but at that point you'll be at a REALLY long single line and still a possibility of eventually hitting the last case. Unless you want to ternary the second example into a single line:

const isEmpty = val => (val == null) ? true : ('length' in val) ? !val.length : ('size' in val) ? !val.size : !Object.keys(val).length;

This does however now fail for primitives:

isEmpty(1) // TypeError
isEmpty('1') // TypeError
isEmpty(false) // TypeError

[skatcat31]

also this works on functions, which... is that the right way to do things?

isEmpty(() => null) // true

Maybe we should go with a mini library? Check if it's a primitive and if so return true, otherwise proceed onto tests for iterator, length, and then keys?

[haoliangwu]

maybe this repo should contain other specific type check util methods like isMap、isSet and etc.

Then we can ensure val as Map/Set type by invoking (isMap || isSet) methods and check if its size property equal to zero without increasing time complexity.

[skatcat31]

@haoliangwu we decided against that a long time ago because we want each snippet to be able to be used on their own. This is why this one is kind of monster

[ChakshuGautam]

also this works on functions, which... is that the right way to do things?

isEmpty(() => null) // true

Maybe we should go with a mini library? Check if it's a primitive and if so return true, otherwise proceed onto tests for iterator, length, and then keys?

@skatcat31
Something like adding this
val !== Object(val) ? true
for the primative check. Its quite long but, most of these functions go into the utils module in my projects.

``` Javascript
const isEmpty = val => (val == null) ? true : val !== Object(val) ? true : ('length' in val) ? !val.length : ('size' in val) ? !val.size : !Object.keys(val).length;

[ghost]

i would like to work on the project

[skatcat31]

Maybe we just adapt this to a more generic "check property" snippet?

const checkProp = (predicate, property) => obj => predicate(obj[property])

[Chalarangelo]

@skatcat31 Maybe add a disclaimer to the current snippet and also add the one you are suggesting under utility. I think it's a pretty useful one!

[skatcat31]

I keep forgetting to do this... After a long day at work I often just boot up a game now...

[skatcat31]

@Chalarangelo after checking the current state I'm going to close this. As it notes it checks for keys length or object property length it should be obvious this would not work on something this is defined empty by a size property. While some may not understand it, it means they aren't looking closely enough at the Map and Set types. Why Map and Set don't have lengths is beyond me... Either add a size to Arrays or keep it consistent TC39...

[GrosSacASac]

The current description is still

Returns true if the a value is an empty object, collection, map or set, has no enumerable properties or is any type that is not considered a collection.

The original reason Map and Set use size instead of length is for backwards compatibility, with old libraries that check for length

[skatcat31]

good catch. Will open PR

[skatcat31]

Opened PR to update language and remove Map and Set tests

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.