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

Combinators for n-ary functions (use apply instead of funcall) #72

Closed
wants to merge 1 commit into from

Conversation

vapniks
Copy link

@vapniks vapniks commented Feb 9, 2014

N-ary versions of -on, -not, -orfn, -andfn
New combinators -rotate-args and -reverse-args for rotating/reversing
arglist of n-ary functions.

Altered items:
-on2, -rotate-args, -reverse-args, -not2, -orfn2, -andfn2,

N-ary versions of -on, -not, -orfn, -andfn
New combinators -rotate-args and -reverse-args for rotating/reversing
arglist of n-ary functions.

Altered items:
-on2, -rotate-args, -reverse-args, -not2, -orfn2, -andfn2,
@vapniks
Copy link
Author

vapniks commented Feb 9, 2014

You might want to replace the original -on, -not, -orfn, -andfn with the new versions

@magnars
Copy link
Owner

magnars commented Feb 9, 2014

Thanks! I'll ping @Fuco1 for this one.

@Fuco1
Copy link
Collaborator

Fuco1 commented Feb 9, 2014

Personally I find &rest and apply a bit meh and would prefer more explicit applicative style, but of course that's not easily possible in elisp.

The main issue here, however, is speed. Funcall is slow as it is but apply is much much slower. I did a quick benchmark calling

(-sort (-on '< 'string-to-int) numbers)
(-sort (-on2 '< 'string-to-int) numbers)

3000 times on a list of 1000 strings (all files byte-compiled) and the results are

-sort + -on  dash-test      1           7.989396416   7.989396416
-sort + -on2 dash-test      1           30.05434008   30.05434008

This is definitely something you could feel in the result product. I suspect the rest would be equally slowed down.

I can't imagine right now a use case for "ternary -on", but I'm sure there is. For the sake of speed, I'd therefore suggest we keep both variants. the -blabla2 versions could be named -blablan maybe? (to show they are n-ary), and it would be nice to add some note to docstring about the possible performance drawback, e.g. "for binary use, consider faster `-bla'".

Also we should add the types to all the functions... but I'm not sure how to express variable arguments. Maybe a.... So -on2 would be `(b... -> c) -> (a -> b) -> a... -> c

@vapniks
Copy link
Author

vapniks commented Feb 9, 2014

How about adding fast binary versions with funcall since these are most likely to be used (e.g. (-orfn2 'string< 'string=) which is what prompted me to write those functions).
We could name the binary versions -orfn2, -andfn2, etc. and name the n-ary versions -orfnn, -andfnn, etc.

@Fuco1
Copy link
Collaborator

Fuco1 commented Feb 9, 2014

What you mean adding? The present functions all use funcall when possible. -on is mostly useful as binary combinator, -orfn and -andfn as unary (all filters take unary predicates)

Edit: Oh, you mean like adding binary, ternary, ... using funcall and then a generic one for n-ary use? Dunno if that wouldn't be too much noise.....

@vapniks
Copy link
Author

vapniks commented Feb 10, 2014

or how about an extra parameter to indicate the arity? (implemented as macros)

@Fuco1 Fuco1 modified the milestone: 2.14.0 Nov 9, 2016
@basil-conto basil-conto modified the milestones: 2.16.0, 2.19.0 Feb 15, 2021
@basil-conto basil-conto self-assigned this Feb 15, 2021
@basil-conto basil-conto added the enhancement Suggestion to improve or extend existing behavior label Feb 15, 2021
@basil-conto basil-conto removed their assignment Feb 15, 2021
@basil-conto basil-conto added this to In Progress in Release 2.19.0 Mar 9, 2021
@basil-conto basil-conto moved this from In Progress to Blocking in Release 2.19.0 Jun 29, 2021
@basil-conto basil-conto moved this from Blocking to To Do in Release 2.19.0 Jun 29, 2021
@basil-conto basil-conto moved this from To Do to In Progress in Release 2.19.0 Jul 6, 2021
basil-conto added a commit that referenced this pull request Jul 8, 2021
This is a rewrite and extension of PR #72 to address issue #306.
For discussion, especially wrt performance, see PRs #72 and #308.

* dash.el (-on, -flip, -not, -orfn, -andfn): Return a variadic
function.  Declare as pure and side-effect-free.
(-rotate-args): New combinator suggested by @vapniks in PR #72.
(-const): Declare as pure and side-effect-free.

* NEWS.md (2.19.0): Announce -rotate-args and variadic combinators.

* dev/examples.el (-partition-after-pred): Fix oddp bug waiting to
happen with negative dividends.
(-cons*): Check that &rest args are safe to mutate.
(-on, -flip, -const, -not, -orfn, -andfn): Extend tests.
(-rotate-args): New test.

* README.md:
* dash.texi: Regenerate docs.
@basil-conto basil-conto moved this from In Progress to Done in Release 2.19.0 Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Suggestion to improve or extend existing behavior
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants