Add is.any() and is.all() methods #19

kodie · 2017-10-06T17:17:14Z

Here is the any() and all() methods mentioned in #1.

I wasn't 100% sure how you envisioned it working but the way I did it was both methods accepts two arrays as parameters so that it's possible to check multiple variables for multiple types.

Let me know if you were thinking something different!

melvin0008 · 2017-10-06T17:58:34Z

@kodie At the bottom of the issue sindresorhus explained his idea of any and all. You can check it out. For eg. any(is.string, {}, true, '🦄');

kodie · 2017-10-06T18:00:46Z

@melvin0008 oh shoot! Completely missed that. I will update my PR.

melvin0008 · 2017-10-06T18:07:59Z

Sure.
It will something on the lines of this:

is.any = (func, ...values) => values.some(func);

is.all = (func, ...values) => values.every(func);

kodie · 2017-10-06T18:19:21Z

PR Updated. 🍻

kodie · 2017-10-06T18:23:49Z

ah, using rest parameters break Node v4 support. :/

kodie · 2017-10-06T19:19:52Z

There we go 👍

gioragutt · 2017-10-06T19:24:15Z

index.js

@@ -169,4 +169,30 @@ const isEmptyMapOrSet = x => (is.map(x) || is.set(x)) && x.size === 0;

 is.empty = x => !x || isEmptyStringOrArray(x) || isEmptyObject(x) || isEmptyMapOrSet(x);

+is.any = function (predicate, values) {


See Array.prototype.some for the predicate check.

Take the values as an array using (predicate, ...values) => { ... } instead of the slice.

Use arrow functions instead of an anonymous function. All other methods use this syntax. You have to be consistent with the rest of the code.

basically, I think the code can look like is.any = (predicate, ...values) => values.some(predicate);
of course, you have to validate the values is an array using is.array.

I now see that ...values is impossible, So as I've said, put a comment stating the it's not possible due to node4 compatibility.

So keep the current logic, but use the array methods like before, and extract a common method to which you can pass the correct method (aka Array.prototype.some or Array.prototype.every).

gioragutt · 2017-10-06T19:35:37Z

index.js

@@ -169,4 +169,30 @@ const isEmptyMapOrSet = x => (is.map(x) || is.set(x)) && x.size === 0;

 is.empty = x => !x || isEmptyStringOrArray(x) || isEmptyObject(x) || isEmptyMapOrSet(x);

+is.any = function (predicate, values) {
+	let ret = false;
+	values = Array.prototype.slice.call(arguments, 1);


Generally, just a note, don't use arguments, ES5 has given has enough ways to deprecate usage pf arguments.

If you really end up having to use arguments, make sure you document it well.

@gioragutt What would I use instead of arguments if I can't use ...values?

Notice the comment I've made above. I wrote it before I've went over the commits.

@gioragutt oh, gotcha!

gioragutt · 2017-10-06T19:36:28Z

index.js

+	return ret;
+};
+
+is.all = function (predicate, values) {


Array.prototype.every, same as before.

gioragutt · 2017-10-06T19:39:36Z

test.js

@@ -402,3 +402,17 @@ test('is.empty', t => {
 	tempSet.add(1);
 	t.false(m.empty(tempSet));
 });
+
+test('is.any', t => {


Missing test cases:

no items (aka m.any(m.integer))

invalid predicate? what if the passed predicate is not a function? this should be validated

@sindresorhus say is.function(predicate) === false, do we throw an Error?

say is.function(predicate) === false, do we throw an Error?

Yes

gioragutt · 2017-10-06T19:39:46Z

test.js

+	t.false(m.any(m.integer, true, 'lol', {}));
+});
+
+test('is.all', t => {


same as above

kodie · 2017-10-06T20:17:43Z

@gioragutt PR Updated again, let me know what you think.

gioragutt · 2017-10-06T20:24:08Z

index.js

@@ -169,4 +169,35 @@ const isEmptyMapOrSet = x => (is.map(x) || is.set(x)) && x.size === 0;

 is.empty = x => !x || isEmptyStringOrArray(x) || isEmptyObject(x) || isEmptyMapOrSet(x);

+const isAnyOrAll = (all, predicate, values) => {


Instead of passing all as a boolean, pass in a method param instead.

this would actually be either Array.prototype.some or Array.prototype.every,

and you'd call method.call(values, predicate).

it makes sense more than passing a boolean. If we're speaking clean code and SOLID, this violates both Single Responsibility Principle and Open Close Principle, because it makes isAnyOrAll (which should be renamed, for the same reasons) responsible for both having the the calls to the methods inside of it, and needing the logic to decide which method is called.

By passing the method from the caller, we can allow more methods to use isAnyOrAll.

I'm sure that there are more method of Array that receive a predicate.

Makes sense! What would you suggest we change the name of isAnyOrAll to?

predicateOnArray

gioragutt · 2017-10-06T20:26:23Z

index.js

+	}
+
+	if (values.length === 0) {
+		throw new TypeError(`Invalid number of values: ${util.inspect(values.length)}`);


You can never really have a value other than 0, so why call inspect on this? instead, make the error more readable, f.e: No values passed to ${callerMethod}, where callerMethod can be passed as an argument.

gioragutt · 2017-10-06T20:27:29Z

index.js

+
+// We have to use anonymous functions for the any() and all() methods
+// to get the arguments since we can't use rest parameters in node v4.
+is.any = function (predicate) {


is.any = predicate => isAnyOrAll(...)

Have to use an anonymous function in order to get the correct arguments object. For some reason the arguments object is weird with arrow functions.

God I hate old js.

https://stackoverflow.com/questions/30935336/official-information-on-arguments-in-es6-arrow-functions

Arrow functions dont have arguments.

Just for the sake of debugging, name the anonymous functions (isAll and isAny) so that they'll be named if they appear in a stack trace.

After that, I'll approve

Also, we now have is.empty for length === 0

Just for the sake of debugging, name the anonymous functions (isAll and isAny) so that they'll be named if they appear in a stack trace.

That is not necessary. JS engines are able to infer the name.

Also, we now have is.empty for length === 0

I think it's overdoing it using is.empty for that.

kodie · 2017-10-07T00:13:12Z

@gioragutt Updated.

gioragutt · 2017-10-07T00:46:57Z

index.js

+
+// We have to use anonymous functions for the any() and all() methods
+// to get the arguments since we can't use rest parameters in node v4.
+is.any = function (predicate) {


Just for the sake of debugging, name the anonymous functions (isAll and isAny) so that they'll be named if they appear in a stack trace.

After that, I'll approve

gioragutt · 2017-10-07T00:48:52Z

index.js

+
+// We have to use anonymous functions for the any() and all() methods
+// to get the arguments since we can't use rest parameters in node v4.
+is.any = function (predicate) {


Also, we now have is.empty for length === 0

gioragutt

@sindresorhus If so, then this PR is done?

sindresorhus · 2017-10-11T09:26:54Z

👌😎

gioragutt · 2017-10-11T10:31:33Z

@sindresorhus great! v0.3.0?

sindresorhus · 2017-10-11T10:33:27Z

Already done an hour ago ;)

Add is.any() and is.all() methods

ce367d3

Correct is.any() and is.all() methods

23b1e22

Fix Node v4 support for any() and all() methods

c523b95

gioragutt requested changes Oct 6, 2017

View reviewed changes

gioragutt mentioned this pull request Oct 6, 2017

Use TypeScript #9

Closed

Clean up is.any() and is.all() code. Throw in some validation

bf32335

gioragutt requested changes Oct 6, 2017

View reviewed changes

Rename isAnyOrAll to predicateOnArray, clean up some more code

c8632ac

gioragutt requested changes Oct 7, 2017

View reviewed changes

gioragutt approved these changes Oct 7, 2017

View reviewed changes

sindresorhus merged commit 651f434 into sindresorhus:master Oct 11, 2017

		@@ -169,4 +169,30 @@ const isEmptyMapOrSet = x => (is.map(x) \|\| is.set(x)) && x.size === 0;

		is.empty = x => !x \|\| isEmptyStringOrArray(x) \|\| isEmptyObject(x) \|\| isEmptyMapOrSet(x);

		is.any = function (predicate, values) {

		@@ -169,4 +169,35 @@ const isEmptyMapOrSet = x => (is.map(x) \|\| is.set(x)) && x.size === 0;

		is.empty = x => !x \|\| isEmptyStringOrArray(x) \|\| isEmptyObject(x) \|\| isEmptyMapOrSet(x);

		const isAnyOrAll = (all, predicate, values) => {

Add is.any() and is.all() methods #19

Add is.any() and is.all() methods #19

Conversation

kodie commented Oct 6, 2017

melvin0008 commented Oct 6, 2017

kodie commented Oct 6, 2017

melvin0008 commented Oct 6, 2017 • edited

kodie commented Oct 6, 2017

kodie commented Oct 6, 2017

kodie commented Oct 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodie Oct 6, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodie commented Oct 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodie commented Oct 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gioragutt left a comment

Choose a reason for hiding this comment

sindresorhus commented Oct 11, 2017

gioragutt commented Oct 11, 2017

sindresorhus commented Oct 11, 2017

melvin0008 commented Oct 6, 2017 •

edited

kodie Oct 6, 2017 •

edited