-
Notifications
You must be signed in to change notification settings - Fork 69
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
Headers for the new-style DiscreteTrajectory #3137
Conversation
physics/discrete_trajectory2.hpp
Outdated
template<typename Frame> | ||
class DiscreteTrajectory2 : public Trajectory<Frame> { | ||
public: | ||
using Iterator = DiscreteTrajectoryIterator<Frame>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
physics/discrete_trajectory2.hpp
Outdated
Iterator end() const; | ||
|
||
Iterator rbegin() const; | ||
Iterator rend() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should hope the reverse iterator is not equal to the iterator, otherwise the container cannot contain more than one element.
std::reverse_iterator<iterator>
, which should probably be named using reverse_iterator = std::reverse_iterator<iterator>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
physics/discrete_trajectory2.hpp
Outdated
SegmentIterator segments_end() const; | ||
|
||
SegmentIterator segments_rbegin() const; | ||
SegmentIterator segments_rend() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as for rbegin, std::reverse_iterator<SegmentIterator>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
physics/discrete_trajectory2.hpp
Outdated
Iterator upper_bound(Instant const& t) const; | ||
|
||
SegmentIterator segments_begin() const; | ||
SegmentIterator segments_end() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to iterate over the segments, a SegmentRange
type accessible as trajectory.segments()
would be useful, so that we can write
for (auto const& segment: trajectory.segments()) {
rather than
for (auto segments_it = trajectory.segments_begin();
segments_it != trajectory.segments_end();
++segments_it) {
auto const& segment = *segments_it;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding segments()
and rsegments()
since it avoids all these segment_
functions.
SegmentIterator segments_rbegin() const; | ||
SegmentIterator segments_rend() const; | ||
SegmentRange segments() const; | ||
ReverseSegmentRange rsegments() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Right Thing to do is to have only segments
and use an adapter to iterate backward (in C++20 that would be https://en.cppreference.com/w/cpp/ranges/reverse_view, maybe absl has a thing in the meantime, or we can write one in base
, we should have make_reverse_iterator
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a TODO to fix this once we have C++20.
First step of #3136.