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

Return type consistency #17

Open
philipbl opened this issue Dec 14, 2016 · 3 comments
Open

Return type consistency #17

philipbl opened this issue Dec 14, 2016 · 3 comments

Comments

@philipbl
Copy link
Owner

Currently, this is how the queue functions when it is empty:

assert self.queue.peek(items=0) == []
assert self.queue.peek(items=1) is None
assert self.queue.peek(items=2) == []
assert self.queue.peek(items=100) == []

assert self.queue.get(items=0, block=False) == []
with pytest.raises(queue.Empty):
    self.queue.get(items=1, block=False)

When the queue is has data in it, the following is true:

assert isinstance(self.queue.get(items=0), list)
assert isinstance(self.queue.get(items=1), int)
assert isinstance(self.queue.get(items=2), list)

assert isinstance(self.queue.peek(items=0), list)
assert isinstance(self.queue.peek(items=1), int)
assert isinstance(self.queue.peek(items=2), list)

It seems like the return values are slightly inconsistent. It makes sense that when peeking and items == 1 a value is returned or None if there are no items in the queue, and when items > 1 a list is returned. What should be the behavior for items == 0?

I propose that when items == 0, None is returned for both peek and get operations, rather than an empty list.

This means that peek can return 3 types of values: None, a value, and a list. get can return 4 types of values: None, a value, a list, and an Empty exception. That seems like a lot of different return types.

@Kriechi do you have any thoughts on this?

@Kriechi
Copy link
Contributor

Kriechi commented Dec 14, 2016

Yes this is a very tricky issue. Considering #9, returning None is also troublesome...
I guess we first have to prohibit None from being used as item value. Then we can use it as return value.

@philipbl
Copy link
Owner Author

#16 fixes #9 without prohibiting None. I leave it up to the user to figure out if the None is from the queue or because there is nothing in the queue. If the user wants to distinguish, they can check the size of the queue.

@Kriechi
Copy link
Contributor

Kriechi commented Dec 14, 2016

Checking the size and calling get or peek has an inherent race condition - if the user doesn't use our internal mutex.

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