view functor generation #153
Conversation
include/seqan3/range/view/detail.hpp
Outdated
| // be e.g. "int" and not "int &&", but we really want the tuple in the functor | ||
| // to always only contain lvalue or rvalue references | ||
| // [adding rvalue reference to lvalue reference is NOP which we want] | ||
| return {std::add_rvalue_reference_t<arg_types>(args)...}; |
There was a problem hiding this comment.
I found this last-minute, before it generated dangling stack references if the argument type inside the view was &.
😅
There was a problem hiding this comment.
hm, or maybe I didnt fix it 🤔
bc9e2a4 to
fd7f4fc
Compare
542d19f to
075b1d3
Compare
|
ok, done from my POV |
| /*************++ without args ************************/ | ||
|
|
||
| // not actually a view | ||
| template <typename urng_t> |
There was a problem hiding this comment.
sry for the stupid question, but what is a urange? what does the u stand for?
There was a problem hiding this comment.
TL;DR: u stands for underyling range. It is the range object this views operates on.
Since view functors operate on another range and return a new range (more precisely a view), I used to call the types the input range (irng_t irange) and the output range. However this is confusing, because input range and output range are actually concepts that describe certain characteristics of the range, not whether they are returned from a functor or put into one...
So the range returned is actually also a seqan3::input_range_concept and not a seqan3::output_range_concept. Seeing that this was confusing I already renamed the "output range" to "returned range" in our documentation.
I now discussed with @rrahn that we also rename the input to "underlying range". This is a TODO for the existing views, but in this PR and #154, well as our Tutorial already adapt this naming.
|
Ah okay. underlying_range_t is to long? Rng is for me a random number generator. But I had a hinge that you mean ranges. I imagine that using range_t is to under specified. If we stick to urng_t this should be explained within each doc for the template argument. I found it only in one.
Thank you for clarification
Am 3. März 2018 12:27:07 MEZ schrieb Hannes Hauswedell <notifications@github.com>:
…h-2 commented on this pull request.
> +//
+//
==========================================================================
+
+#include <iostream>
+#include <memory>
+
+#include <gtest/gtest.h>
+
+#include <seqan3/range/view/detail.hpp>
+
+using namespace seqan3;
+
+/*************++ without args ************************/
+
+// not actually a view
+template <typename urng_t>
TL;DR: u stands for **underyling** range. It is the range object this
views operates on.
Since view functors operate on another range and return a new range
(more precisely a view), I used to call the types the input range
(`irng_t irange`) and the output range. However this is confusing,
because input range and output range are actually concepts that
describe certain characteristics of the range, not whether they are
returned from a functor or put into one...
So the range returned is actually also a seqan3::input_range_concept
and not a seqan3::output_range_concept. Seeing that this was confusing
I already renamed the "output range" to "returned range" in our
documentation.
I now discussed with @rrahn that we also rename the input to
"**underlying** range". This is a TODO for the existing views, but in
this PR and #154, well as our Tutorial already adapt this naming.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#153 (comment)
--
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
|
include/seqan3/range/view/detail.hpp
Outdated
|
|
||
| //!\brief Helper function to unpack the tuple. | ||
| template <typename urng_t, size_t... Is> | ||
| auto explode(urng_t && urange, std::index_sequence<Is...> const &) |
There was a problem hiding this comment.
Just minor thing. This function should be scoped in a protected region.
Not really an exposable interface.
The same holds for the arguments member.
075b1d3 to
859b545
Compare
859b545 to
0f37a47
Compare
|
@rrahn I think I addressed all your concerns. For the arguments member of the intermediate functor type I have renamed it |
|
lgtm |
fixes #142