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

sg14::ring_span: clear () member function gone awol #165

Open
degski opened this issue Jun 8, 2019 · 2 comments
Open

sg14::ring_span: clear () member function gone awol #165

degski opened this issue Jun 8, 2019 · 2 comments

Comments

@degski
Copy link

degski commented Jun 8, 2019

Maybe I'm missing a point, but having a clear() member function seems rather useful [popping them one by one vs. doing it O(1)] to me. Iff the sg14::ring_span would be assignable, I could do without, but that doesn't seem to be the case either. It is assignable, but not how I tried to do that. So if I have a sg14::ring_span as a class member, the only option is to pop till it drops :-) .

@Quuxplusone
Copy link
Contributor

For ring_span<T, copy_popper<T>> and ring_span<T, null_popper<T>>, you could theoretically add a clear() member function that just reset the "begin" and "end" of the span. For ring_span<T, default_popper<T>>, such a member would be confusing unless it also called the popper on each existing element of the span.

Consider

std::vector<std::unique_ptr<Widget>> v(10);
sg14::ring_span<std::unique_ptr<Widget>> r(v.begin(), v.end());
r.push_back(std::make_unique<Widget>());
r.push_back(std::make_unique<Widget>());
r.clear();  // this needs to call ~Widget twice, somehow, right?

If you really want to leave the contents of the underlying array untouched, then you could just use the assignment operator:

std::vector<std::unique_ptr<Widget>> v(10);
sg14::ring_span<std::unique_ptr<Widget>> r(v.begin(), v.end());
r.push_back(std::make_unique<Widget>());
r.push_back(std::make_unique<Widget>());
r = sg14::ring_span<std::unique_ptr<Widget>>(v.begin(), v.end());

With that new information, does clear() still appeal as a solution?

@degski
Copy link
Author

degski commented Jun 9, 2019

@Quuxplusone Thanks for responding.

I was looking at this implementation. I tried assignment, but then the compiler balked, not, when I do it the way you posted. So, yes that is a solution.

Having said that. Because we have people [probably not SG14's target group] who might want to put std::unique_ptr held objects is a ring_span others [who might just have a buffer of pods or aggregates of pods] have to pay a high price [higher than necessary, i.e. it violates "you don't pay for what you don't use"] for that [setting m_size to zero ( with clear() ) should do]. Equally, all STL-containers support clear(), and now we have a 'view' and all of a sudden we cannot use clear(), this seems silly at best and is the world upside-down. Iff I have pods in an array, the should just wilt into the distance at no cost.

I think the design needs more work, because of this. I understand that unique_ptr's cause ownership issues (in this case, normally they try to solve them), but let's start and make this work well and as good as possible for pods (i.e. trivially_destructable and what have you). It needs to work well for all cases, though, as there is no alternative std::ring_buffer in the STL (and Boost.Circular Buffer appears to be C++03).

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