Feature/std23 stride #26
Conversation
| std::conditional_t< | ||
| std20::ranges::bidirectional_range< Base >, | ||
| std20::bidirectional_iterator_tag, | ||
| std20::forward_iterator_tag > >; | ||
|
|
There was a problem hiding this comment.
This is just a style change, yes? Is the goal to be consistent in this style? Can I put it like this:
if items inside < > extend more than 1 entry, then use a line break and add one entry per new line?
If this is the objective for consistent styling, template on line 80 needs to also be adjusted
There was a problem hiding this comment.
I did this mainly for readability (the conditional is like a ternary statement). The style of these standard-like pieces of code is not as fixed as I would like it. I basically have not decided if we should use our normal style or something more like the nanorange style.
| @@ -0,0 +1,139 @@ | |||
| // include Catch2 | |||
| #include <catch2/catch_test_macros.hpp> | |||
There was a problem hiding this comment.
Can we add some more tests to this? The above is pretty complicated templating. While it looks like ok from a cursory look, I think it is very possible I mis-read the logic in a place or three. I'd like to see things like striding where we don't end on the last value, for instance. I get that this is temporary addition that will be replaced on adopting c++23 and we don't want to go too overboard, but testing more scenarios would increase my confidence that this is working as intended.
There was a problem hiding this comment.
A valid point. And it did not work as it should have in this state. I actually added a test case later on to verify if these would work in our use cases in ENDFtk, see #27 for that test. I found multiple bugs because of it which are also corrected in that PR.
There was a problem hiding this comment.
We also should add more tests on the iterator operations (some of these are used under the hood in the view_interface functions but they most likely do not cover everything).
In preparation for replacing range-v3 in ENDFtk, we need a few pieces defined in the c++23 standard: chunk, stride, zip, etc.
This adds the following: