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

Add ring_span::push_front(), emplace_front(), pop_back() ? #99

Closed
martinmoene opened this issue May 8, 2017 · 6 comments
Closed

Add ring_span::push_front(), emplace_front(), pop_back() ? #99

martinmoene opened this issue May 8, 2017 · 6 comments

Comments

@martinmoene
Copy link

See Arthur O'Dwyer's reference implementation std::ring_span.

@martinmoene martinmoene changed the title Add push_front(), emplace_front() ? Add push_front(), emplace_front(), pop_back() ? May 8, 2017
@martinmoene martinmoene changed the title Add push_front(), emplace_front(), pop_back() ? Add ring_span::push_front(), emplace_front(), pop_back() ? May 8, 2017
@hatcat
Copy link
Member

hatcat commented May 18, 2017

I'm conflicted about adding API that would defeat the FIFO nature of the ring. I can see how they are "missing" from the interface when other containers are taken in to account, but the point of the ring is that you put something in to it, and then you take it out again, in order. The addition of these functions makes the interface easier to use incorrectly. Do you have some use cases for interfering with a ring in this way?

@martinmoene
Copy link
Author

I'm conflicted about adding API that would defeat the FIFO nature of the ring.

The word 'ring' inherently does not convey any meaning with respect to a beginning or an ending.

Has the name fifo_span ever been considered?

@martinmoene
Copy link
Author

So then, among others, there are the forces of symmetry/asymmetry and easy to use/hard to misuse.

@Quuxplusone
Copy link
Contributor

My opinion is that we ought to provide them since they're cheap. (I can't think of anything that would make them expensive for any implementation to provide.) I'll note that our ring<T>::iterators right now are BidirectionalIterators, not ForwardIterators, which implies that people ought to be able to iterate in reverse (#100, #107), which is related to this issue — this issue is about not merely "traversing backwards", but actually "mutating backwards".

I do wonder whether something special ought to happen if you try to push_front onto a ring that is already full. Should it really overwrite the back element of the ring and decrement the end pointer? This does feel somehow awkward; but it is perfectly symmetrical with push_back.

@mattreecebentley
Copy link
Contributor

I don't really have a lot of input here, as inexperienced as I am with ring buffers, but the only thing I'd note is whatever behaviour is added should ideally follow the principle of least surprise, for end-user. M

@hatcat
Copy link
Member

hatcat commented Jun 6, 2017

I'm not sure that cheap is a good enough reason to trump increased possibility of misuse, but perhaps we can let the committee decide.

@hatcat hatcat closed this as completed Jun 6, 2017
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

4 participants