Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify anyPass and allPass for common use case #989

Closed
wants to merge 1 commit into from
Closed

Simplify anyPass and allPass for common use case #989

wants to merge 1 commit into from

Conversation

paldepind
Copy link
Member

The current implementation of allPass and anyPass seems a bit odd to me. They support predicates taking a variable amount of arguments.

There is a test case where the predicates are all of different arity? I don't see this making sense since all the predicates are passed the same arguments.

I can't think of any use case where a list of predicates would all take more than one argument, all be of the same arity and all make sense to the same set of arguments?

Furthermore allPass and anyPass are variadic. So even though their arity is 1 I can do stuff like this: R.allPass([gt10, even], 11);. As far as I can see this is undocumented.

This commit does the following: Change allPass and anyPass into functions taking two arguments that behaves exactly like allPass and anyPass does today when the predicates takes one argument. But, they don't support anything else.

This has the advantage that support for R.allPass([gt10, even], 11) can be supported without relying on variadic behavior. The implementation is also vastly simplified.

I realize that I should have opened an issue before creating this pull request in case there actually is a realistic use case for the removed functionality.

@CrossEye
Copy link
Member

I have no problem with a PR instead of an issue. In general, I'd rather have something concrete to discuss.

As to the content, there are several other issues that bring up related points about the arities of generated functions. A few recent ones come to mind: #943, #985. #986. I'm starting to think we really need some sort of coordinated approach to them all.

It seems to me that in JS, it's reasonable that the functions might all have an arity different from 1. Obviously it doesn't make much sense if they have different arities, although of course some functions might get passed parameters they don't use. That makes me think that the correct signature for these functions is

allPass(fns.., args..)
anyPass(fns.., args..)

But that makes the simplest case, that of passing a single argument, noticeably worse. And I'm not thrilled by that.

@paldepind
Copy link
Member Author

I have no problem with a PR instead of an issue. In general, I'd rather have something concrete to discuss.

It was mostly for the sake of my own time. There'd be no reason to write code if you didn't like the idea from the get go.

It seems to me that in JS, it's reasonable that the functions might all have an arity different from 1.

How? They should all be predicate functions, all have the same arity and all make sense for the same arguments. I could be wrong, but I don't seem practical to me. All the examples are with functions taking one argument.

But that makes the simplest case, that of passing a single argument, noticeably worse. And I'm not thrilled by that.

Yes. That I think is the alternative. But as you mentioned, it makes the common case worse.

I can think of lots of cases where doing multiple checks on a single value is useful. Here's just one from the top of my head:

var validateUsernameLength = R.allPass([gt8, lt20]);
var validateAge = R.allPass([validAge, ageOver18]);
var validateUserObj = R.allPass([validateUsername, validatePassword, validateAge]);

But, I can't think of anything where you'd want to send several values through a series of functions. But, if such a use case exists I think your last proposal is the way to go – out of necessity.

@paldepind
Copy link
Member Author

It just occurred to me, if you have a use case for the current behavior you could simply do this (using the new implementation):

R.allPass(R.map(R.apply, [list, of, functions]), [a, bunch, of, arguments]);

And you'd still get the short circuiting. That doesn't seem like a great lose to me for a rare use case when it keeps the main use case simple.

Oh, and note that my suggestion completely avoids the problem of the arity of the returned function since it doesn't return a new function at all.

@CrossEye CrossEye mentioned this pull request Mar 31, 2015
var arity = require('../arity');
var max = require('../max');
var __ = require('./__');
var _curry3 = require('./internal/_curry3');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the build is failing because of the "internal" here.

@CrossEye
Copy link
Member

As to uses, I know that I've had some. Here's one possibility: Imagine a convex polygon in the plane with vertices at (2, 4), (5, 7), (9, 5), (7, 2), and (4, 1). One good way to test whether an arbitrary (x, y) value is inside the polygon is with

var inPolygon = R.allPass([
  (x, y) => x - y > -2,
  (x, y) => x + 2 * y < 19,
  (x, y) => 3 * x - 2 * y < 17,
  (x, y) => x - 3 * y < 1,
  (x, y) => 3 * x + 2 * y > 14
]);

inPolygon(5, 3); //=> true
inPolygon(6, 7); //=> false

Yes, there are other ways this could be done, including wrapping the parameters in some sort of Point objects, but this is quite clean and very clear.

I'm sure I could come up with other examples if I tried.

@paldepind
Copy link
Member Author

One could argue that you're actually only passing one entity with two properties. But, it is a use case.

I see three options

  1. Keep it as is. This is the most powerful option but also the most complex one.
  2. Take the parameters in an array. This makes the function invariadic but it is also pretty awkward to call the function returned after passing one argument to allPass/anyPass.
  3. My proposal. This results in a very simple function but with the cost of some lost power.

Of these I prefer the first or the third depending on how often the use case of using predicates taking multiple arguments occurs (I'm still not really convinced).

@paldepind
Copy link
Member Author

Btwn, I've borked the content of this PR win a fight with Git :(

I can reconstruct it though if necessary :)

@buzzdecafe
Copy link
Member

((:bell:))

@davidchambers
Copy link
Member

The source is showing up as unknown repository, so I don't think this pull request can be revived. @paldepind could open a new one, though. :)

@buzzdecafe buzzdecafe closed this May 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants