feat(collections): Returns an array excluding single given value

[getspooky]

Returns an array excluding all given values

Example:

without([2, 1, 2, 3, 2], 1,2);
// 3

Discussion

Please see #1065

[kt3k]

Please add test cases when excluded has more than LARGE_ARRAY_SIZE items.

[kt3k]

BTW this covers both without withoutAll cases because this without accepts any number or arguments. @LionC Do you agree with this design?

[LionC]

@kt3k

BTW this covers both without withoutAll cases because this without accepts any number or arguments. @LionC Do you agree with this design?

I would personally prefer to distingush without and withoutAll. The current design forces spreading for all withoutAll cases, which does not feel too nice to me and may have performance implications as well.

@getspooky would it be ok for you to split this into two functions?

[LionC]

It might make sense to build a Set for the values to exclude, as that reduces the runtime from N * V to N + V at the cost of a bit of memory for the Set.

Something like:

const valuesToExclude = new Set(values)

return array.filter(it => !valuesToExclude.has(it))

[getspooky]

Thanks @kt3k and @LionC for feedback,
I don't see the purpose of making tow functions without and withoutAll that do the same task.
it would be more easy and efficient to make one function without that take n of args.

[LionC]

Thanks @kt3k and @LionC for feedback,

I don't see the purpose of making tow functions without and withoutAll that do the same task.

it would be more easy and efficient to make one function without that take n of args.

While they do the same task on an abstract level, a simple without can be optimized differently, while a withoutAll can take an Array instead of varargs, which saves the user the need to ...spread every time they use the functions, saving some performance as well (...spreading is not free).

As all the functions in this module are pretty low level and attract usage in other algorithms, we should emphasize even small-ish performance gains. We have done that for all other functions in this module to this point.

I would also challenge the assumption that both functions are the same in the mind of users - removing a single element and removing an arbitrarily long list of elenents can be quite different use cases and happen in different frequencies.

[getspooky]

@kt3k , @LionC I just fix without to accept one value, i will open withoutAll in separated PR.

[kt3k]

@getspooky Thanks for updating! The implementation looks good to me now.

@LionC
After seeing this one liner implementation:

return array.filter((element) => value !== element);

I'm now skeptical about the benefit of providing this function as part of std/collections. The above is straightforward way to do this and I don't know why the users want to use this without function over simple array.filter((e) => e !== v).

[timreichen]

I'm now skeptical about the benefit of providing this function as part of std/collections. The above is straightforward way to do this and I don't know why the users want to use this without function over simple array.filter((e) => e !== v).

I would recommend against implementing this function. An abstraction of a one-liner is not necessary imo. Using array.filter directly is more flexible.

[LionC]

I'm now skeptical about the benefit of providing this function as part of std/collections. The above is straightforward way to do this and I don't know why the users want to use this without function over simple array.filter((e) => e !== v).

I would recommend against implementing this function. An abstraction of a one-liner is not necessary imo. Using array.filter directly is more flexible.

reduce is more flexible than filter. for is more flexible than reduce.

As stated several times, I do not see the harm in having such a function, while there is gain in it for some people, even if that does not include everyone. Other libraries have choosen to include it even though they have filter as well and people use it - because it states their intent clearly, is easily scannable and short.

I feel like discussions like this boil down to coding style (how much abstraction / renaming do you like) a lot. We should keep in mind that deno does not try to make decisions like that for its users.

[LionC]

@kt3k I think we should try to get a call on this, otherwise this PR will stand still forever.

I personally do not see the harm in having a clearly named function doing a common useful thing with a concise API.

Also, this(general) or this(specific) SO question and the (sometimes horrible) accepted/upvoted answers make me think that there is value in having a clearly named function with an optimal implementation...

[kt3k]

@LionC
From my experience of collaborating with many people with various levels, even very beginners can write this filtering by themselves. So I highly doubt this is useful for somebody.

And many collaborators (Luca, Nayeem, Divy,...) are strongly against adding 'too trivial' items into std/collections (and we already retracted many items in earlier iteration because of the triviality of them). I'm afraid this one typically falls into that category.

[kt3k]

Talked a little bit with @LionC, but we're closing this particular one (mainly because of the triviality).

@getspooky Thank you for working on this anyways.