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

Functions should be bound to context or not use this #2

Open
Yomguithereal opened this issue Mar 17, 2017 · 2 comments
Open

Functions should be bound to context or not use this #2

Yomguithereal opened this issue Mar 17, 2017 · 2 comments

Comments

@Yomguithereal
Copy link

Hello @acarl005. The fact that your functions need this to function has this somewhat uncomfortable side-effect that you cannot use them without calling them from the generatorics object.

Here is what I mean:

// If you do the following for instance:
var combination = require('generatorics').combination;

// This will throw & fail because the function does not have the required scope
combination([1, 2, 3], 2);

A solution would be to bind your function to the correct scope before exporting or rewrite the code marginally not to rely on the scope of the generatorics object.

@Yomguithereal
Copy link
Author

On a side note, decomposing functions further would also mean someone could import only parts of the library as a result:

var combination = require('generatorics/combination');

I'll be happy to draft a PR if you want.

@acarl005
Copy link
Owner

@Yomguithereal Thanks for the feedback! You're absolutely right. I've always found it annoying when I have to manually re-bind methods to their context. The API could certainly be better. A PR would be greatly appreciated!

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

2 participants