Refactored buffer and buffer_view #74
Conversation
include/sparrow/buffer_view.hpp
Outdated
| namespace sparrow | ||
| { | ||
| /* | ||
| * @class buffer_view |
There was a problem hiding this comment.
You can remove this and rely on the autobrief syntax?
There was a problem hiding this comment.
Not sure, I will set up a skeleton for the doc and check whether we can do it in a dedicated PR. If so, I will remove all of them.
| using const_reverse_iterator = std::reverse_iterator<const_iterator>; | ||
|
|
||
| explicit buffer_view(buffer<T>& buffer); | ||
| buffer_view(pointer p, size_type n); |
There was a problem hiding this comment.
To be complete we also need a subrange(...) function and a constructor that takes a pair of iterators (they should be template parameters constrained by concept, not buffer_view::iterator).
These are often used with view types (see std::span, which also provides first and last subview functions).
There was a problem hiding this comment.
Maybe we can do that in a dedicated PR? This piece of code was just moved from the initial buffer.hpp file. I'll add a todo for that.
There was a problem hiding this comment.
I feel that this and the iterator pair constructor (which should be what the subrange is built with) not being there makes it incomplete. But ok, it can be added in another PR although please do it immediately after this one then?
| * @brief Object that references but does not own a piece of contiguous memory | ||
| */ | ||
| template <class T> | ||
| class buffer_view |
There was a problem hiding this comment.
How is this type different from std::span?
There was a problem hiding this comment.
They are very similar indeed. However, std::span is missing some APIs in C++20 (like cbegin / cend ), and we may want to provide additional std;:vector like APIs.
There was a problem hiding this comment.
Could you add that clarification in the comments, so that we keep a trace in the code?
Fix #45