Skip to content

Conversation

@MathieuDutSik
Copy link
Contributor

Motivation

The QueueView has front / delete_front while we may want to have a pop.
The SetView also has the contains / remove, and we may want to do both at once.

Proposal

Implement a .pop() for QueueView. Implement a remove_if_present for SetView.

Unfortunately, the front is not systematically followed by a delete_front. So, we cannot use it in the code.
For the SetView change, another PR indicates a different direction: Altogether removal of the unskippable entries.

Test Plan

CI. Unit tests have been added.

Release Plan

Not that important. Could be

Links

None.

@MathieuDutSik MathieuDutSik changed the title Add the functionality to the QueueView / SetView. Addf functionality to the QueueView / SetView. Dec 10, 2025
@MathieuDutSik MathieuDutSik changed the title Addf functionality to the QueueView / SetView. Add functionality to the QueueView / SetView. Dec 10, 2025
@MathieuDutSik MathieuDutSik marked this pull request as ready for review December 10, 2025 12:59
Comment on lines +294 to +297
pub async fn pop(&mut self) -> Result<Option<T>, ViewError> {
let value = self.front().await?;
self.delete_front();
Ok(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out no. That being said it is the standard usage of queues.

@bart-linera
Copy link
Contributor

Instead of adding remove_if_present, wouldn't it make more sense for remove to return a boolean, indicating whether the value was present? That's how BTreeSet and HashSet work, at least.

@MathieuDutSik
Copy link
Contributor Author

Instead of adding remove_if_present, wouldn't it make more sense for remove to return a boolean, indicating whether the value was present? That's how BTreeSet and HashSet work, at least.

We could of course do that. But fn remove is not async and does not even return an error. So, it is quite a useful idea to still have both forms even if their name could change from the ones we have now.

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.

3 participants