WG21-SG14 / SG14

A library for Study Group 14 of Working Group 21 (C++)
501 stars 53 forks source link

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

Open degski opened 5 years ago

degski commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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).