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

Function length behavior #30

Open
rtm opened this issue Oct 21, 2015 · 18 comments
Open

Function length behavior #30

rtm opened this issue Oct 21, 2015 · 18 comments

Comments

@rtm
Copy link
Contributor

rtm commented Oct 21, 2015

I note that in the lodash implementation of curry

Note: This method does not set the "length" property of curried functions.

So someone, at least, thinks that is not necessary. And I cannot really think of use cases where it is. Of course we want to be able to find the length of functions like function add(a, b), but in what cases do we need to know the length of add::curry()(1)?

I raise this point because the util-arity approach introduces another layer of nested function which will have a performance impact (I don't like the way util-arity is implemented, either, but that's a separate issue), it introduces extraneous elements into stack traces, and so on.

Then again, the https://github.com/dominictarr/curry library does maintain arity.

Do we really need to maintain arity on the result of currying a function?

@stoeffel
Copy link
Collaborator

We discussed the arity of curried functions in #17.

This method does not set the "length" property of curried functions.

I don't like lodash's implementation of curry for exactly that reason. IMO in FP it's important to be explicit and that includes that a function has a fixed arity.

Then again, the https://github.com/dominictarr/curry library does maintain arity.

and Ramda's curry does maintain arity too.

which will have a performance impact

does this really have a big impact on performance? Do you have a benchmark?

// cc @tomekwi

related #29

@davidchase
Copy link
Member

Theres actual a good article about arity-and-partial-function-application by raganwald.

The usage of util-arity maybe revisited if implementation isnt ideal.

But i would like to also see benchmarks to discuss performance and i can understand a possiblity of it being an issue somewhat, however, i currently use ramda in production at work and ramda as mentioned by @stoeffel maintains arity and i have not noticed too many performance issues.. yes we know its not as fast as lodash and there are talks about improving performance but not a cost the maintaining arity.

If needed I can provide the discussions from the ramda repository as well

@rtm
Copy link
Contributor Author

rtm commented Oct 21, 2015

Arity is the number of arguments a function "expects". Consider

function foo(a, b, c) { return a + b + c; }
var curried_foo = foo::curry();
var foo1 = curried_foo(1);

So what is the arity of foo1?

foo1 can be called with no arguments, which is sort of a no-op, or one argument, which returns another curryable function, or two arguments, which invokes the underlying foo. In other words, foo1 has no specify arity. It can be called with different numbers of arguments, which is the definition of "variadic".

Now of course we can legitimately ask the question: "What is the number of additional, non-placeholder arguments that would need to be passed to foo1, in order to complete the currying process and call the value of the underlying function? That is the number 2. However, that is a different concept from the "arity" of foo1, which, as I mentioned, is variadic. If that is considered something real important for people to have access to, we could give them access to a property remaining_argcount on the function.

A typical example of things one wants to do with arities (fn.length) is to use them in higher-order functions to do things like writing wrappers to enforce argument counts:

function argchecked(fn) {
  return function() {
    assert(fn.length === arguments.length);
    return fn.apply(this, arguments);
  };
}

But this is precisely something that we would never want to do with curried functions, since they can legitimately be called with any number of arguments.

This is different from partially applied functions, where it makes perfect sense to report the arity of the result of partial application. In other words, in the following case,

function foo(a, b, c) { return a + b + c; }
var partial_foo = foo::partial(1);

it is correct to report the arity of partial_foo as 2.

Whether or not curry-this incurs any substantial performance penalty by imposing an arity on the results of currification, I don't know. Perhaps it's minor. But it seems logically wrong and not worth the trouble or possible performance hit.

@stoeffel
Copy link
Collaborator

Whether or not curry-this incurs any substantial performance penalty by imposing an arity on the results of currification, I don't know

Would be cool, if we could see some benchmarks. I want have time to do that today, but maybe someone (or you) would like to create a PR and compare the current master to your performance branch.

@tomek-he-him
Copy link
Member

foo1 can be called with no arguments, which is sort of a no-op, or one argument, which returns another curryable function, or two arguments, which invokes the underlying foo. In other words, foo1 has no specify arity. It can be called with different numbers of arguments, which is the definition of "variadic".

Good reasoning!

But it seems logically wrong and not worth the trouble or possible performance hit.

The decision came because of the partial application functionality that came bundled with curry-this.

But the more I think about it (and read your posts @rtm), the more I prefer fn::partiallyApply(_, 2, 3, _)::curry() or fn::shave(3)::curry() to fn::curry(_, 2, 3, _). partiallyApply or shave sets fn.length, and curry uses this length to curry the function.

See #29 (comment) and #29 (comment).

@stoeffel
Copy link
Collaborator

The decision came because of the partial application functionality that came bundled with curry-this.

The logical move would be to separate the partial application functionality to it's own module partial-this.

@tomek-he-him
Copy link
Member

separate the partial application functionality to it's own module

👍

shave-this wouldn’t be bad too. For simpler use cases. I prefer ::shave(4) than ::partial(_, _, _, _).

@stoeffel
Copy link
Collaborator

shave-this wouldn’t be bad too. For simpler use cases. I prefer ::shave(4) than ::partial(_, _, _, _).

There is a home for both modules now. and we can remove the functionality from curry-this as soon as @thisables/partial and @thisables/shave are ready (contributions are more than welcome).

@rtm
Copy link
Contributor Author

rtm commented Oct 23, 2015

So "thisables" is an organization? I'm a little fuzzy on how they work. Do they group repos together? What does it mean to be a member of an organization? Is your intent to make shave a separate repo?

Maybe it's just me, but I'm not a big fan of micro-repositories. We already have a bunch of machinery in curry-this for testing and what not which would need to be replicated. People would have to npm install multiple packages into their apps, etc. What are the benefits you see from individual repos, if that's what you have in mind?

@davidchase
Copy link
Member

So "thisables" is an organization? I'm a little fuzzy on how they work. Do they group repos together?

Yes grouping modules with a common functionality IMHO is a great way to make them discoverable (good way to get more contributors involved, etc)

Also allows them to have separate issues/discussions and PRs.

People would have to npm install multiple packages into their apps, etc

Hmm i not sure what you mean here? if you have module A that depends on module B, module C, etc it would be pulled in automatically when requiring from npm. It would be the same if those modules were built into the core and not as separate repos.

What are the benefits you see from individual repos, if that's what you have in mind?

I think Sindre Sorhus does a great job explaining his take on the subject as he a prolific developer with tons of small modules. read here

Again thought I would share my thoughts... @stoeffel @tomekwi @hemanth may have different even contrasting opinions.

@rtm
Copy link
Contributor Author

rtm commented Oct 24, 2015

I read Sindre's post. I want to make sure I'm understanding what is being proposed. Is it

$ npm install @thisable/curry
$ npm install @thisable/shave
import {curry} from '@thisable/curry';
import {shave} from '@thisable/shave'; 

or something else?

Personally, I'd rather do

$ npm install @thisable/curry
import {curry, shave} from '@thisable/curry';

I'm not quite following your point about

if you have module A that depends on module B, module C, etc it would be pulled in automatically when requiring from npm

Is A here meant to be a user module, or some top-level curry module which re-exports individual routines from individual modules it specifies as dependencies?

@stoeffel
Copy link
Collaborator

I totally agree with @davidchase. Was about to link to sindre's answer as well.
@rtm yop one would have to install shave and curry if one needs both.

@rtm
Copy link
Contributor Author

rtm commented Oct 24, 2015

Not sure if the group is interested in discussing this further, but what is being proposed is roughly equivalent to Underscore putting each of its APIs into a separate package/repo.

Sindre's rationale does not exactly apply here. He has a bunch of little semi-independent utilities. Sure, in that case publish them as separate mini-repos, rather than in one big kitchen-sink repo. We and Underscore), on the other hand, have a group of related APIs.

There are two sets of "users". One is the end-users, the other the developers. From the standpoint of the end-users, AIUI in the proposed structure we will be asking them to add a separate dependency for each module they want to use, and if they want to use both curry and shave, to write two import statements from two different modules. If the refactor their code and no longer need shave, they should ideally uninstall it. They have multiple readmes to read. Their versions of the two modules might get out of whack.

From the standpoint of us developers, multiple micro-repos means we have multiple repos to run tests in, multiple repos to handle PRs against, multiple repos to publish to npm, and multiple repos to handle issues about.

On the other hand, I am having trouble thinking of any downsides to a single repo. One might choose separate repos if there were management/access control issues, which I don't think exist here. A user using e.g. browserify will bring only those individual pieces he or she needs, so there are no payload size issues. What other negatives are there?

@tomek-he-him
Copy link
Member

@rtm, I used to share your opinion on this subject – but a discussion with @mathdesl led me to change my mind. It’s not as easy to read as a blog post, but it does present good arguments on both sides: mattdesl/module-best-practices#2.

By the way, multiple independent packages don’t always need to be multiple repos – as babel and parametric.svg (among others) show us.

The setup of a multi-module monorepo is quite complicated in itself though – it might make sense with 1-liners but I’m not convinced of the benefits here.

@rtm
Copy link
Contributor Author

rtm commented Oct 25, 2015

I will defer to wiser minds on this topic.

I assume you have seen https://blog.andyet.com/2015/01/07/modularizing-underscorejs. He has implemented quite a bit of interesting machinery to manage his micro-repositories, including ways to create consolidated documentation, update licenses across all of the one-liners, etc. I assume we also intend to do something like that eventually?

@rtm
Copy link
Contributor Author

rtm commented Oct 26, 2015

@tomek-he-him
Copy link
Member

If multiple packages are actually dependent on one another – would be developed in parallel / should have common docs / common tests / etc. – then a monorepo has lots of benefits! I’ve done this a couple of times now and it speeds up development by an order of magnitude!

But it only makes sense with a single ecosystem of packages – and not when a bunch of packages are independent on one another. If thisables should become something like 1-liners then a monorepo is definitely the way to go.

Where thisables should head long-term is rather a question to @stoeffel though.

@stoeffel
Copy link
Collaborator

But it only makes sense with a single ecosystem of packages – and not when a bunch of packages are independent on one another.

Agreed.

If thisables should become something like 1-liners then a monorepo is definitely the way to go.

I don't intend thisables to become some thing like 1-liners. Thisables should be just a container for different modules which make use of the function bind operator.

btw. I created a repo for shave and partial. If any one wants to start, feel free to do so 😄

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

No branches or pull requests

4 participants