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

extra parameter to item.task()? #2

Closed
dooglus opened this issue May 29, 2013 · 3 comments
Closed

extra parameter to item.task()? #2

dooglus opened this issue May 29, 2013 · 3 comments
Labels

Comments

@dooglus
Copy link

dooglus commented May 29, 2013

Hey, fancy seeing you here!

I'm just learning node.js and came across your semaphore.js. Looking at the code, I'm wondering why you're passing semaphore.leave to item.task, when item.task doesn't take any parameters.

Not doing so seems to work. Is this a better way to do things?

diff --git a/lib/semaphore.js b/lib/semaphore.js
index 534ec18..da50b13 100644
--- a/lib/semaphore.js
+++ b/lib/semaphore.js
@@ -25,7 +25,7 @@ module.exports = function(capacity) {
            }

            semaphore.current += item.n;
-           item.task(semaphore.leave);
+           item.task();
        },

        leave: function(n) {
@abrkn
Copy link
Owner

abrkn commented May 30, 2013

It can:

var sem = require('semaphore')()
sem.take(function(leave) {
    console.log('about to leave')
    leave()
})

@abrkn abrkn closed this as completed May 30, 2013
@dooglus
Copy link
Author

dooglus commented May 30, 2013

The one the user provides can, but the one you're calling can't:

item.task = function() { task(semaphore.leave); };

[...]

item.task(semaphore.leave);

You've replaced the user's task (which can take a parameter) with a new function that can't.

@abrkn
Copy link
Owner

abrkn commented May 31, 2013

I don't understand the issue. Submit a pull request if there is a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants