Skip to content

Possible p() & v() confusion #13

New issue

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

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

Already on GitHub? # to your account

Closed
nhat-nguyen opened this issue Mar 3, 2018 · 2 comments
Closed

Possible p() & v() confusion #13

nhat-nguyen opened this issue Mar 3, 2018 · 2 comments
Assignees
Labels

Comments

@nhat-nguyen
Copy link

Looking at the source code and the examples, it seems to me that the library is using p() to signal/free up a resource, whereas v() is used to wait/"block" on the semaphore.

However, according to the definition of a semaphore, the uses of p() and v() are the other way around. Some resources to verify are:

Please let me know if I'm misunderstanding the use case here. :)

@OlliV
Copy link
Collaborator

OlliV commented Mar 4, 2018

Yes you are absolutely correct, the traditional meaning of the terms is unfortunately mixed in this library. However if you think the letters could mean V = verhogen = increase and P = passeren = pass, then we can build a meaningful explanation for the function names. As you probably have noted the implementation is based on a fixed number of tokens distributed to the consumers. Now v() increases the number of registered consumers (either returns a token or puts the consumer to a queue waiting for a token), and p() clearly passes the token back to the Semaphore and eventually to the next waiter in the queue.

Retrospectively thinking I should have probably never used single letter function names anyway but something like acquire() and release(), that would have been a lot more self-explanatory.

@OlliV OlliV added the question label Mar 4, 2018
@nhat-nguyen
Copy link
Author

Thanks for the confirmation. Any chance the API would be updated with more standard names?

@OlliV OlliV self-assigned this Mar 6, 2018
OlliV added a commit that referenced this issue Mar 11, 2018
The function names `v()` and `p()` in this library were
accidentally reversed from the tradition. Since the function names
might be a bit confusing even if named correctly, we should rename
them as `acquire()` and `release()` but also provide the old names
for backwards compatibility.

Fixes #13
@leo leo closed this as completed in #14 Mar 18, 2018
leo pushed a commit that referenced this issue Mar 18, 2018
* New function names for clarity

The function names `v()` and `p()` in this library were
accidentally reversed from the tradition. Since the function names
might be a bit confusing even if named correctly, we should rename
them as `acquire()` and `release()` but also provide the old names
for backwards compatibility.

Fixes #13

* Missed this

* Add deprecation warnings

* Fix ts definitions
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants