Skip to content

[ fix ] BoundedQueue1#69

Merged
stefan-hoeck merged 4 commits into
idris-community:mainfrom
Matthew-Mosior:fix-boundedqueue1-to-make-thread-safe-refactor
Mar 29, 2026
Merged

[ fix ] BoundedQueue1#69
stefan-hoeck merged 4 commits into
idris-community:mainfrom
Matthew-Mosior:fix-boundedqueue1-to-make-thread-safe-refactor

Conversation

@Matthew-Mosior

Copy link
Copy Markdown
Collaborator

This PR addresses comments from #66.

@Matthew-Mosior

Copy link
Copy Markdown
Collaborator Author

@stefan-hoeck

I've addressed all comments in #66 here in this PR, it's ready for review.

@stefan-hoeck

Copy link
Copy Markdown
Member

Code looks good, but I have doubts about the semantics. What are the use cases for a bounded queue, that either silently drops the oldest element (data loss in case of enqueue) or silently fails to enqueue a value in the first place (enqueueIfSpace)? Wouldn't it make more sense to return some kind of flag that informs client code about the success or failure of the operation?

@Matthew-Mosior

Copy link
Copy Markdown
Collaborator Author

Code looks good, but I have doubts about the semantics. What are the use cases for a bounded queue, that either silently drops the oldest element (data loss in case of enqueue) or silently fails to enqueue a value in the first place (enqueueIfSpace)? Wouldn't it make more sense to return some kind of flag that informs client code about the success or failure of the operation?

Good point here, I've rewritten the function to return a F1 s Bool instead, should be good for a re-review.

@stefan-hoeck

Copy link
Copy Markdown
Member

Good point here, I've rewritten the function to return a F1 s Bool instead, should be good for a re-review.

But there are several functions that try to add a value to a queue. Now only one of them returns a Bool. They should be treated all the same, no?

@Matthew-Mosior

Copy link
Copy Markdown
Collaborator Author

Good point here, I've rewritten the function to return a F1 s Bool instead, should be good for a re-review.

But there are several functions that try to add a value to a queue. Now only one of them returns a Bool. They should be treated all the same, no?

Yep good catch, prepend needed this fix too. should be good for a re-review.

@stefan-hoeck
stefan-hoeck merged commit 0f7d694 into idris-community:main Mar 29, 2026
2 checks passed
@Matthew-Mosior

Copy link
Copy Markdown
Collaborator Author

Thank you for the review!

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

Successfully merging this pull request may close these issues.

2 participants