1
\$\begingroup\$

This is a follow-up question for A recursive_transform_reduce Function for Various Type Arbitrary Nested Iterable Implementation in C++, A recursive_count Function For Various Type Arbitrary Nested Iterable Implementation in C++, A recursive_count_if Function For Various Type Arbitrary Nested Iterable Implementation in C++, A Summation Function For Boost.MultiArray in C++, An arithmetic_mean Function For Various Type Arbitrary Nested Iterable Implementation in C++, A recursive_transform Template Function with Execution Policy and A population_variance Function For Various Type Arbitrary Nested Iterable Implementation in C++. Thanks to G. Sliepen's answer. I am trying to perform the suggestion that avoiding the requires clause if possible by using the concept name instead of class in the template parameter list. As the result, the improved version of recursive_count function, recursive_count_if function, recursive_size function, recursive_reduce function, arithmetic_mean function, recursive_transform function, recursive_transform_reduce function and population_variance function are as below.

template<typename T>
concept is_back_inserterable = requires(T x)
{
    std::back_inserter(x);
};

template<typename T>
concept is_elements_iterable = requires(T x)
{
    std::begin(x)->begin();
    std::end(x)->end();
};

template<typename T1, typename T2>
concept is_std_powable = requires(T1 x1, T2 x2)
{
    std::pow(x1, x2);
};

template<typename T>
concept is_summable = requires(T x) { x + x; };

//  recursive_count implementation
template<std::ranges::range T1, class T2> requires (!is_elements_iterable<T1>)
constexpr auto recursive_count(const T1& input, const T2 target)
{
    return std::count(input.begin(), input.end(), target);
}

//  transform_reduce version
template<is_elements_iterable T1, class T2>
constexpr auto recursive_count(const T1& input, const T2 target)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [target](auto& element) {
        return recursive_count(element, target);
        });
}

//  recursive_count_if implementation
template<std::ranges::range T1, class T2> requires (!is_elements_iterable<T1>)
constexpr auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::count_if(input.begin(), input.end(), predicate);
}

//  transform_reduce version
template<is_elements_iterable T1, class T2>
constexpr auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [predicate](auto& element) {
        return recursive_count_if(element, predicate);
        });
}

//  recursive_size implementation
template<class T> requires (!std::ranges::range<T>)
constexpr auto recursive_size(const T& input)
{
    return 1;
}

template<std::ranges::range T> requires (!is_elements_iterable<T>)
constexpr auto recursive_size(const T& input)
{
    return input.size();
}

template<is_elements_iterable T>
constexpr auto recursive_size(const T& input)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [](auto& element) {
        return recursive_size(element);
        });
}

//  recursive_reduce implementation
template<class T, class ValueType, class Function = std::plus<ValueType>>
constexpr auto recursive_reduce(const T& input, ValueType init, const Function& f)
{
    return f(init, input);
}

template<std::ranges::range Container, class ValueType, class Function = std::plus<ValueType>>
constexpr auto recursive_reduce(const Container& input, ValueType init, const Function& f = std::plus<ValueType>())
{
    for (const auto& element : input) {
        auto result = recursive_reduce(element, ValueType{}, f);
        init = f(init, result);
    }
    return init;
}

template<typename T>
concept is_recursive_reduceable = requires(T x)
{
    recursive_reduce(x, T{});
};

template<typename T>
concept is_recursive_sizeable = requires(T x)
{
    recursive_size(x);
};

//  arithmetic_mean implementation
template<class T = double, is_recursive_sizeable Container>
constexpr auto arithmetic_mean(const Container& input)
{
    if (recursive_size(input) == 0) //  Check the case of dividing by zero exception
    {
        throw std::logic_error("Divide by zero exception"); //  Handle the case of dividing by zero exception
    }
    return (recursive_reduce(input, T{})) / (recursive_size(input));
}

//  recursive_transform implementation
template<class T, class F>
constexpr auto recursive_transform(const T& input, const F& f)
{
    return f(input);
}

template<class T, std::size_t S, class F>
constexpr auto recursive_transform(const std::array<T, S>& input, const F& f)
{
    using TransformedValueType = decltype(recursive_transform(*input.cbegin(), f));

    std::array<TransformedValueType, S> output;
    std::transform(input.cbegin(), input.cend(), output.begin(), 
        [f](auto& element)
        {
            return recursive_transform(element, f);
        }
    );
    return output;
}

template<template<class...> class Container, class Function, class... Ts>
requires (is_back_inserterable<Container<Ts...>> && std::ranges::range<Container<Ts...>> && !is_elements_iterable<Container<Ts...>>)
// non-recursive version
constexpr auto recursive_transform(const Container<Ts...>& input, const Function& f)
{
    using TransformedValueType = decltype(f(*input.cbegin()));
    Container<TransformedValueType> output;
    std::transform(input.cbegin(), input.cend(), std::back_inserter(output), f);
    return output;
}

template<template<class...> class Container, class Function, class... Ts>
requires (is_back_inserterable<Container<Ts...>> && is_elements_iterable<Container<Ts...>>)
constexpr auto recursive_transform(const Container<Ts...>& input, const Function& f)
{
    using TransformedValueType = decltype(recursive_transform(*input.cbegin(), f));
    Container<TransformedValueType> output;

    std::transform(input.cbegin(), input.cend(), std::back_inserter(output),
        [&](auto& element)
        {
            return recursive_transform(element, f);
        }
    );

    return output;
}

#ifdef USE_BOOST_MULTIDIMENSIONAL_ARRAY
template<is_multi_array T, class F>
constexpr auto recursive_transform(const T& input, const F& f)
{
    boost::multi_array output(input);
    for (decltype(+input.shape()[0]) i = 0; i < input.shape()[0]; i++)
    {
        output[i] = recursive_transform(input[i], f);
    }
    return output;
}
#endif

//  With execution policy
template<class ExPo, class T, class F>
constexpr auto recursive_transform(ExPo execution_policy, const T& input, const F& f)
{
    return f(input);
}

template<class ExPo, class T, std::size_t S, class F>
requires std::is_execution_policy_v<std::remove_cvref_t<ExPo>>
constexpr auto recursive_transform(ExPo execution_policy, const std::array<T, S>& input, const F& f)
{
    using TransformedValueType = decltype(recursive_transform(*input.cbegin(), f));

    std::array<TransformedValueType, S> output;
    std::transform(execution_policy, input.cbegin(), input.cend(), output.begin(),
        [execution_policy, f](auto& element)
        {
            return recursive_transform(execution_policy, element, f);
        }
    );
    return output;
}

template<class ExPo, template<class...> class Container, class Function, class... Ts>
requires (std::is_execution_policy_v<std::remove_cvref_t<ExPo>> && std::ranges::range<Container<Ts...>> && !is_elements_iterable<Container<Ts...>>)
// non-recursive version
constexpr auto recursive_transform(ExPo execution_policy, const Container<Ts...>& input, const Function& f)
{
    using TransformedValueType = decltype(f(*input.cbegin()));
    Container<TransformedValueType> output(input.size());
    std::transform(execution_policy, input.cbegin(), input.cend(), output.begin(), f);
    return output;
}

template<class ExPo, template<class...> class Container, class Function, class... Ts>
requires (std::is_execution_policy_v<std::remove_cvref_t<ExPo>> && is_elements_iterable<Container<Ts...>>)
constexpr auto recursive_transform(ExPo execution_policy, const Container<Ts...>& input, const Function& f)
{
    using TransformedValueType = decltype(recursive_transform(*input.cbegin(), f));
    Container<TransformedValueType> output(input.size());

    std::transform(execution_policy, input.cbegin(), input.cend(), output.begin(),
        [&](auto& element)
        {
            return recursive_transform(execution_policy, element, f);
        }
    );

    return output;
}

#ifdef USE_BOOST_MULTIDIMENSIONAL_ARRAY
template<class ExPo, is_multi_array T, class F>
requires (std::is_execution_policy_v<std::remove_cvref_t<ExPo>>)
constexpr auto recursive_transform(ExPo execution_policy, const T& input, const F& f)
{
    boost::multi_array output(input);
    for (decltype(+input.shape()[0]) i = 0; i < input.shape()[0]; i++)
    {
        output[i] = recursive_transform(execution_policy, input[i], f);
    }
    return output;
}
#endif

//  recursive_transform_reduce implementation
template<class Input, class T, class UnaryOp, class BinaryOp = std::plus<T>>
constexpr auto recursive_transform_reduce(const Input& input, T init, const UnaryOp& unary_op, const BinaryOp& binop = std::plus<T>())
{
    return binop(init, unary_op(input));
}

template<std::ranges::range Input, class T, class UnaryOp, class BinaryOp = std::plus<T>>
constexpr auto recursive_transform_reduce(const Input& input, T init, const UnaryOp& unary_op, const BinaryOp& binop = std::plus<T>())
{
    return std::transform_reduce(std::begin(input), std::end(input), init, binop, [&](auto& element) {
        return recursive_transform_reduce(element, T{}, unary_op, binop);
        });
}

template<typename T>
concept can_calculate_variance_of = requires(const T & value)
{
    (std::pow(value, 2) - value) / std::size_t{ 1 };
};

template<typename T>
struct recursive_iter_value_t_detail
{
    using type = T;
};

template <std::ranges::range T>
struct recursive_iter_value_t_detail<T>
    : recursive_iter_value_t_detail<std::iter_value_t<T>>
{ };

template<typename T>
using recursive_iter_value_t = typename recursive_iter_value_t_detail<T>::type;

//  population_variance function implementation (with recursive_transform_reduce template function)
template<class T = double, is_recursive_sizeable Container>
requires (can_calculate_variance_of<recursive_iter_value_t<Container>>)
constexpr auto population_variance(const Container& input)
{
    if (recursive_size(input) == 0) //  Check the case of dividing by zero exception
    {
        throw std::logic_error("Divide by zero exception"); //  Handle the case of dividing by zero exception
    }
    auto mean = arithmetic_mean<T>(input);
    return recursive_transform_reduce(std::execution::par,
        input, T{}, [mean](auto& element) {
        return std::pow(element - mean, 2);
        }, std::plus<T>()) / recursive_size(input);
}

A Godbolt link is here.

All suggestions are welcome.

The summary information:

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Because this update seems primarily based on the changes to the concepts and requires clauses, that’s what I’m going to focus on.

is_do_thingable is an anti-pattern.

This is really an abuse of the idea of concepts:

template<typename T>
concept is_back_inserterable = requires(T x)
{
    std::back_inserter(x);
};

// ...

template<typename T1, typename T2>
concept is_std_powable = requires(T1 x1, T2 x2)
{
    std::pow(x1, x2);
};

If you find yourself writing do_thingable concepts for every do_thing() function, I’d say you’re really missing the point of concepts. The point of concepts is not to dig out the guts of your algorithm, take every do_x, do_y, and do_z operation out of the body, and then write a requires clause that repeats that entire body as requires do_xable<T> && do_yable<T> && do_zable<T>. The point is to consider what kind of type your algorithm is supposed to work for. It’s about SEMANTICS, not syntax.

For example, instead of is_std_powable<T, U>, you (probably) want something like floating_point<T> and (floating_point<U> or integral<U>). Or even better arithmetic<T>. Or maybe just number<T>. The point is, the concept is not about what you’re doing to the T, it’s about what that T is conceptually supposed to be (note the key word: “CONCEPTually”). If you’re raising it to a power, it’s probably supposed to be a number. If it’s not a number, then even though it might “work” with std::pow() (for example, you can raise true to the power of false)… it’s probably not what you want. bool works syntactically with std::pow()… but probably not semantically for any real-use purpose.

I think you missed the point of the advice.

It looks like previous reviews suggested that you “[avoid] the requires clause if possible by using the concept name instead of class in the template parameter list”. That does not mean that you should eliminate ALL requires clauses, or that requires clauses are a bad thing in any way. It just means that instead of:

template <typename Container>
requires std::ranges::range<Container>
auto func(...

you should probably prefer:

template <std::ranges::range Container>
auto func(...

There’s no difference between the two options… except the second is shorter and easier to read, and you’re not repeating the type, so there’s less chance of screwing up.

But this:

template <is_funcable T>
auto func(T)

… is pretty useless. It tells me nothing about what I need for T to be used with func()… except that T can be used with func(). Which… I mean… duh, right?

This is much more useful:

template <typename T>
requires movable<T> and equality_comparable<T>
auto func(T)

That gives me useful information about what types I can use with func(), without spelling out the entire algorithm in the constraints. And the fact that it’s a requires clause (and not a single concept in the template preamble) isn’t a problem at all.

Try to use standard concepts wherever possible

As near as I can tell, the purpose of is_elements_iterable<T> is to test that T is a range, and that each value in T is also a range.

So what is the difference between is_elements_iterable<T> and std::ranges::range<T> and std::ranges::range<std::ranges::range_value_t<T>>? I can’t see any, which means the latter is a better option. (And there are even better options, which I’ll get to later.)

Using existing (especially standard) concepts is especially important with concepts (as opposed to type traits and SFINAE), because of normalization and subsumption. We’ll get to that next.

recursive_count()

Let’s start with the function signatures you have.

template<std::ranges::range T1, class T2> requires (!is_elements_iterable<T1>)
constexpr auto recursive_count(const T1& input, const T2 target)

template<is_elements_iterable T1, class T2>
constexpr auto recursive_count(const T1& input, const T2 target)

Now, for starters, let’s use more descriptive names than T1 and T2. T1 is always going to be a container or view—a range—so why not call it that? (As for T2, that can be anything, so it could just be T.)

So why are you taking target by value? Imagine I have a vector of strings, and I want to count the number of times a particular very long string appears in the vector. If target is being taken by value, then it’s being copied. Why? Does it need to be? I can’t imagine why. Taking it by const& would avoid the unnecessary copy.

So, rewriting your concepts a bit (temporarily), this is what we’ve got so far:

template <typename Range, typename T>
requires std::ranges::range<Range> and (!is_elements_iterable<Range>)
constexpr auto recursive_count(const Range& input, const T& target)

template <typename Range, typename T>
requires is_elements_iterable<Range>
constexpr auto recursive_count(const Range& input, const T& target)

Let’s pull out just the constraints:

requires std::ranges::range<Range> and (!is_elements_iterable<Range>)

requires is_elements_iterable<Range>

is_elements_iterable sorta-kinda implies std::ranges::range (not technically, because of the way is_elements_iterable is written… but practically, yes). So your constraints are really just:

requires (!is_elements_iterable<Range>)

requires is_elements_iterable<Range>

Now this is NOT the way to use constraints. This is what you’d have to do with type traits and SFINAE, because type traits and SFINAE don’t understand ordering or subsumption. But concepts do understand those things. Concepts understands that if you have two functions, and one is unconstrained while the other requires is_elements_iterable, then anything that satisfies is_elements_iterable should use the second function. You don’t need to specifically say that anything that DOESN’T satisfy is_elements_iterable should use the first; concepts already understands that.

Subsumption means that requires foo<T> and bar<T> is more constrained than requires foo<T>, so if something satisfies both the foo and bar constraints, it will choose the first requires clause. If something only satisfies foo, it will choose the second. If something only satisfies bar, or neither foo nor bar, then neither will work.

So what you really want is just:

/* nothing, no constraints or requires clause */

requires is_elements_iterable<Range>

… and this will do pretty much the exact same thing as what you’ve got.

Well, almost. You also want to constrain the range, so:

requires std::ranges::range<Range>

requires std::ranges::range<Range> and is_elements_iterable<Range>

And you don’t just want a range, you want an input range (because you’re only reading the elements, not writing them), so:

requires std::ranges::input_range<Range>

requires std::ranges::input_range<Range> and is_elements_iterable<Range>

And as mentioned earlier, you can replace is_elements_iterable with standard concepts like so:

requires std::ranges::input_range<Range>

requires std::ranges::input_range<Range> and std::ranges::input_range<std::ranges::range_value_t<Range>>

This is a good set of constraints. Anything that’s not an input range won’t work with either function. Anything that is will work with the first one. Anything that is AND whose values are also input ranges will work with the second.

Now let’s put those back with the rest of the function signatures:

template <typename Range, typename T>
requires std::ranges::input_range<Range>
constexpr auto recursive_count(const Range& input, const T& target)

template <typename Range, typename T>
requires std::ranges::input_range<Range> and std::ranges::input_range<std::ranges::range_value_t<Range>>
constexpr auto recursive_count(const Range& input, const T& target)

Finally, let’s move as much of those constraints into the template prologue, for readability:

template <std::ranges::input_range Range, typename T>
constexpr auto recursive_count(const Range& input, const T& target)

template <std::ranges::input_range Range, typename T>
requires std::ranges::input_range<std::ranges::range_value_t<Range>>
constexpr auto recursive_count(const Range& input, const T& target)

That gives you two constrained function templates, both requiring that the first argument is an input range.

(You could also constrain the second type by making a type trait that gets the recursive value type… probably called recursive_range_value_t<Range>, and use that with std::equality_comparable_with.)

Now, as for the implementations:

template <std::ranges::input_range Range, typename T>
constexpr auto recursive_count(const Range& input, const T& target)
{
    return std::count(input.begin(), input.end(), target);
}

This seems wrong. You require that the input is a range… but that only means that it supports std::ranges::begin(input) and std::ranges::end(input). It does NOT mean it supports input.begin() or input.end(). For example, C arrays work with std::ranges::begin(array) and std::ranges::end(array), but not with array.begin() or array.end(). Your concepts require std::ranges::begin(input) and std::ranges::end(input), so that’s what you should use.

Of course, if you’re doing that, you might as well use std::ranges::count() rather than std::count(), unless you need the execution policy.

template <std::ranges::input_range Range, typename T>
requires std::ranges::input_range<std::ranges::range_value_t<Range>>
constexpr auto recursive_count(const Range& input, const T& target)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [target](auto& element) {
        return recursive_count(element, target);
        });
}

Again, your constraints require std::ranges::begin(input) and std::ranges::end(input), not std::begin(input) and std::end(input).

You have yet another unnecessary copy of target in the lambda capture. There’s no reason not to capture it by reference, and avoid a copy.

Also, I always recommend to prefer auto&& unless you have a good reason not to. In this case, auto& or auto const& will USUALLY work… but there are pathological cases where it might fail (like with vector<bool>). auto&& ALWAYS works; it ALWAYS does the right thing.

recursive_count_if()

//  recursive_count_if implementation
template<std::ranges::range T1, class T2> requires (!is_elements_iterable<T1>)
constexpr auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::count_if(input.begin(), input.end(), predicate);
}

//  transform_reduce version
template<is_elements_iterable T1, class T2>
constexpr auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [predicate](auto& element) {
        return recursive_count_if(element, predicate);
        });
}

Most of what went for recursive_count() goes for recursive_count_if()

recursive_size()

//  recursive_size implementation
template<class T> requires (!std::ranges::range<T>)
constexpr auto recursive_size(const T& input)
{
    return 1;
}

template<std::ranges::range T> requires (!is_elements_iterable<T>)
constexpr auto recursive_size(const T& input)
{
    return input.size();
}

template<is_elements_iterable T>
constexpr auto recursive_size(const T& input)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [](auto& element) {
        return recursive_size(element);
        });
}

Once again, let’s start with just the constraints:

requires (!std::ranges::range<T>)

requires std::ranges::range<T> and (!is_elements_iterable<T>)

requires is_elements_iterable<T>

Again, this is how you’d use type traits and SFINAE, not concepts. You should take advantage of subsumption:

// no requires clause; unconstrained

requires std::ranges::range<T>

requires std::ranges::range<T> and is_elements_iterable<T>

… and standard concepts (and better type names)…:

// no requires clause; unconstrained

requires std::ranges::input_range<Range>

requires std::ranges::input_range<Range> and std::ranges::input_range<std::ranges::range_value_t<Range>>

… which gives:

template <typename T>
constexpr auto recursive_size(T const&)

template <std::ranges::input_range Range>
constexpr auto recursive_size(Range const&)

template <std::ranges::input_range Range>
requires std::ranges::input_range<std::ranges::range_value_t<Range>>
constexpr auto recursive_size(Range const&)

Couple other things. First, the non-range version just returns 1, which is an int. Thing is, the size type is almost always an unsigned type. You’re going to create surprises if recursive_size(x) returns an unsigned type for ranges and a signed type for non-ranges. You probably want to return something like std::size_t{1}, not just 1.

Second, overload 2 uses input.size(). This is not guaranteed to work for a lot of reasons. std::size() and std::ranges::size() only work for ranges that can figure out their size in constant time (which usually means they have a .size() function… but not always, like for C arrays). So it won’t work for std::forward_list, for example—for those types, you’d have to do std::distance(std::ranges::begin(input), std::ranges::end(input)). The best solution would probably to use if constexpr, and test for std::ranges::sized_range, and then use std::ranges::size() if you can, and std::distance() if you can’t.

recursive_reduce()

Nothing new here. Except… you probably don’t want to use std::plus<ValueType>. You probably just want to use std::plus<> (and you probably want to have it as the default for the first overload). Why? Well, plus<T> means c = a + b where a, b, and c are all the same type (which is T). plus<> means that they can all be different types, and the type of c will be properly deduced.

arithmetic_mean()

//  arithmetic_mean implementation
template<class T = double, is_recursive_sizeable Container>
constexpr auto arithmetic_mean(const Container& input)
{
    if (recursive_size(input) == 0) //  Check the case of dividing by zero exception
    {
        throw std::logic_error("Divide by zero exception"); //  Handle the case of dividing by zero exception
    }
    return (recursive_reduce(input, T{})) / (recursive_size(input));
}

Okay, let’s start with the pointless constraint here. Literally EVERYTHING is “recursive_size-able”. If it’s an input range, it will use overload 2 or 3, and if it’s ANYTHING ELSE it will use overload 1 and return 1. So is_recursive_sizeable is a completely meaningless concept.

(Also, literally everything is “recursive_reduce-able”, though you don’t even use that constraint.)

Ironically, you have the first template parameter unconstrained, and the second constrained, when the second could be literally anything, but the first must be some kind of arithmetic type… and probably a floating point type. I’d recommend the following instead:

template <std::floating_point ReturnType = double, typename T>
constexpr auto arithmetic_mean(T const& input) -> ReturnType

// Note that the return type is explicitly specified. Why? Because otherwise
// it will be the result of:
//    recursive_reduce(input, ReturnType{}) / recursive_size()
// ... which may not be ReturnType (for example, if ReturnType is float, and
// input contains doubles, then the result will be double).

You call recursive_size() twice in the function, but that seems unwise. If your input is a million elements large, recursively, getting the size may not be all that quick. There’s no reason not to call it once and save the result.

Also, this is a very technical note, but this isn’t the most efficient or numerically stable way to calculate the mean. Rather than going through the input twice—once to count and one to sum—you could go through it just once with a pair<ReturnType, size_t> and get the size and the sum together simultaneously. There are also ways to calculate the mean as a running tally, which could avoid unnecessary over- or underflows.

recursive_transform()

recursive_transform() is mostly just more of the same as above.

I should note that rather than using back_inserter, which only supports vector, deque, and list, you could use std::inserter(output, std::ranges::end(output)) to support set, map, hash maps, and more.

HOWEVER…

template<template<class...> class Container, class Function, class... Ts>
requires (is_back_inserterable<Container<Ts...>> && std::ranges::range<Container<Ts...>> && !is_elements_iterable<Container<Ts...>>)
// non-recursive version
constexpr auto recursive_transform(const Container<Ts...>& input, const Function& f)
{
    using TransformedValueType = decltype(f(*input.cbegin()));
    Container<TransformedValueType> output;
    std::transform(input.cbegin(), input.cend(), std::back_inserter(output), f);
    return output;
}

I don’t think this is a good idea at all. I get what you’re trying to do. If Container is a vector<int>, and Function is std::to_string, you want the result to be automatically vector<string>. There are two problems with this:

  1. It doesn’t really work. It will work with the most basic container types, like vector and list… but it won’t work for map. What if I have a map<int, string> and I want to do a transform that lower-cases all the strings and gives me a new map<int, string>? Then this function won’t work. (And I know I said it would work for simple container types like vector, but it won’t even work for that. If you use a std::pmr::vector<int> and to_string(), you won’t get a std::pmr::vector<std::string>, you’ll get a std::vector<std::string>.)
  2. It’s not flexible. What if I want to transform a list<string> into a vector<int>? Can’t do it. What if I want to take a vector<string>, and lower-case all the strings in-place? Can’t do it.

There’s a reason why standard algorithms use output iterators rather than clever tricks like this; output iterators always work, for everything, and they’re infinitely flexible.

recursive_transform_reduce() and population_variance()

More of the same.

Summary

I think you misunderstood the recommendation to put concepts in the template parameter list, rather than the requires clause. I don’t think that was intended to suggest you should avoid requires clauses generally. I think what was meant was EXACTLY what was said: if you already have a concept, and you can put it in the template parameter list rather than the requires clause, then you should do that, just because it simplifies the function signature, and makes it harder to screw up and easier to read. But if you don’t have a concept or you can’t put it in the requires clause (because, it would be unwieldy, or because it’s a conjunction or disjunction), then don’t worry about it.

I also think you misunderstand the purpose of concepts. Concepts are not meant to be simple syntactic checks like is_plusable to check for a + b. If a type doesn’t support operator+, then there’s already a tool to detect that: the compiler; if some code requires operator+, the code will fail to compile for types without it. (If you really want pretty or descriptive error messages, you can always use type traits and static_assert.)

Concepts are meant to be SEMANTIC checks. They won’t protect you from any and all syntactic issues. For example, std::ranges::range will probably be true for a signal transmission class with member functions begin() and end() that begin and end a transmission. If you use a signal transmission with a function like recursive_count()… well, that’s just nonsense, and that’s on you; concepts aren’t meant to save you from nonsense like that (presumably, other aspects of the type system will probably save you, though).

Also, concepts are not like type traits and SFINAE. Concepts understand the idea of “more specialized” in a way that type traits and SFINAE do not. You should take advantage of subsumption, and of constraint normalization, when using concepts.

And finally, don’t try to be too clever with your interfaces. Avoid interfaces that take decisions away from the user (unless those decisions are objectively bad). If an interface is too complex or has too many options, you can always add an overload with a simpler interface that internally calls the more complex interface with sane defaults. Being able to choose what type of container I get as output from a function is a VERY handy ability to have, even if there are common defaults. Not having that ability is not a feature, it’s a problem.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ This is a very good answer. I wonder how much of the OPs code could be replaced with regular algorithms and some kind of recursive iterator, for example instead of recursive_transform_reduce(container, ...) do std::transform_reduce(recursive_begin(container), recursive_end(container), ...). You mention that output iterators always work, however in case of a recursive_transform() for example, you'd have to prepare a recursive destination up front to write to, I don't think a recursive_inserter() would be possible, at least not one that copies the input structure perfectly. \$\endgroup\$
    – G. Sliepen
    Dec 12, 2020 at 19:47
  • \$\begingroup\$ Possible for a specific case, perhaps, but probably not generally. Yeah, I think a recursive iterator or container is a much better idea than a recursive algorithm. A thing that bugged me writing the answer (that I didn’t mention, because I was focused on the concepts) was vector<string> (or vector<vector<string>>)… would you want to iterate over the strings or the chars? Seems to me the answer is almost always going to be the strings. You can’t control that with the algorithm. But with a task-specific container/iterator, you could (and then use all normal algorithms). \$\endgroup\$
    – indi
    Dec 13, 2020 at 15:21
  • 1
    \$\begingroup\$ We already looked at the vector<string> issue in an earlier question, you can make it work in some situations by only recursing while the UnaryOp/BinaryOp you pass to the recursive algorithm cannot be applied on the value type of the container at the current level. I'm not sure it could be made to work with a generic kind of recursive_iterator. \$\endgroup\$
    – G. Sliepen
    Dec 13, 2020 at 16:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.