A recursive_transform Function For Various Type Nested Iterable With std::variant Implementation in C++

Question

This is a follow-up question for A TransformAll Function For Various Type Arbitrary Nested Iterable Implementation in C++. The following code is the improved version based on G. Sliepen's answer. In order to match the conventions of the STL, the function named recursive_transform here uses the is_iterable concept and the is_element_iterable concept. Moreover, the copy operation of the input is avoided by updating [_Func](auto element)->auto into [_Func](auto& element) and the redundant part in this lambda function ->auto has been removed. Although the code is improved, I found that there are some cases which the previous version TransformAll function is hard to deal with. One of those cases is the nested iterable ranges with std::variant. I want to focus on this case, such as std::vector<std::variant<long double>>. First of all, the additional concept is_element_variant is included for determining the type of elements in iterable container is std::variant or not. I think there may be another better implementation to this is_element_variant concept. However, the method I surveyed How to check if template argument is std::variant? isn't deal with this with c++-concepts. I prefer to work with concept here and the experimental code is as below. If there is any suggestion about how to improve this is_element_variant concept, please let me know.

template<typename T>
concept is_element_variant = requires(T x)
{
    x.begin()->index();
    x.begin()->valueless_by_exception();
};

The part of the template function recursive_transform which handle the std::variant structure:

template<class T, class _Fn> requires is_iterable<T> && is_element_variant<T>
static T recursive_transform(const T _input, _Fn _Func);       //  Deal with the iterable case which its element is std::variant

template<class T, class _Fn> requires is_iterable<T> && is_element_variant<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;
    
    std::transform(_input.begin(), _input.end(), returnObject.begin(), 
        [_Func](typename std::iterator_traits<typename T::iterator>::value_type x)->
        typename std::iterator_traits<typename T::iterator>::value_type
        {
            return std::visit([_Func](auto&& arg) -> typename std::iterator_traits<typename T::iterator>::value_type
                    {
                        return _Func(arg);
                    }, x);
        });
    return returnObject;
}

The other parts:

template<typename T>
concept is_iterable = requires(T x)
{
    x.begin();      // must have `x.begin()` 
    x.end();        // and `x.end()` 
};

template<typename T>
concept is_element_iterable = requires(T x)
{
    x.begin()->begin();
    x.end()->end();
};

template<class T, class _Fn> requires is_iterable<T>
static T recursive_transform(const T _input, _Fn _Func);       //  Deal with the iterable case like "std::vector<long double>"

template<class T, class _Fn> requires is_iterable<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;

    std::transform(_input.begin(), _input.end(), returnObject.begin(), _Func);
    return returnObject;
}

template<class T, class _Fn> requires is_iterable<T> && is_element_iterable<T>
static T recursive_transform(const T _input, _Fn _Func);

template<class T, class _Fn> requires is_iterable<T> && is_element_iterable<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;
    std::transform(_input.begin(), _input.end(), returnObject.begin(),
        [_Func](auto& element)
        {
            return recursive_transform(element, _Func);
        }
    );
    return returnObject;
}

int main()
{
    std::vector<long double> testVector1;
    testVector1.push_back(1);
    testVector1.push_back(20);
    testVector1.push_back(-100);
    std::cout << recursive_transform(testVector1, [](long double x)->long double { return x + 1; }).at(0) << std::endl;

    std::vector<long double> testVector2;
    testVector2.push_back(10);
    testVector2.push_back(90);
    testVector2.push_back(-30);

    std::vector<std::vector<long double>> testVector3;
    testVector3.push_back(testVector1);
    testVector3.push_back(testVector2);
    std::cout << recursive_transform(testVector3, [](long double x)->long double { return x + 1; }).at(1).at(1) << std::endl;
    
    std::vector<std::variant<long double>> testVector4;
    testVector4.push_back(1);
    testVector4.push_back(20);
    testVector4.push_back(-100);

    auto operation_to_element = [](long double number) { return number + 2; };

    std::visit([](auto&& arg) {std::cout << arg; },         //  For printing
        recursive_transform(testVector4, operation_to_element).at(0)
    );

    return 0;   
}

All suggestions are welcome.

The summary information:

• Which question it is a follow-up to?

  A TransformAll Function For Various Type Arbitrary Nested Iterable Implementation in C++

• What changes has been made in the code since last question?

  • Rename function to recursive_transform to match the conventions of the STL.
  • The copy operation of the input is avoided by updating auto &element.
  • Remove the redundant part in lambda function ->auto

• Why a new review is being asked for?

  I think the concept is_element_variant may be improved and I am looking forward to any suggestion for possible improvement ways. Moreover, in my opinion of the part of the template function recursive_transform which handle the std::variant structure, implementation here is complex, there are two nested lambda function. If there is any possible to simplify this, please let me know.

Answer 1

You are now making your algorithms more specialized again. Personally, I would avoid this and leave recursively transforming up to recursive_transform(), and handling visiting the variant up to the caller. Perhaps there are ways to make it easier for the caller to do this, but in this answer I'll just comment on your implementation.

Be as precise as possible with your concepts

The concepts you are using should test exactly that which you need. In your code, you are not calling index() nor valueless_by_exception(), so this should not be tested for in the concepts you require. Instead, what you need to test for is whether you can call std::visit() on an element, like so:

template<typename T>
concept is_element_visitable = requires(T x)
{
    std::visit([](auto){}, *x.begin());
};

Simplify the way you write types

Use auto and decltype() where applicable to avoid writing types in a roundabout way. This also has the advantage that you are not requiring that there are proper iterator_traits and other type aliases defined for the containers that might be used. For example:

template<class T, class Fn> requires is_iterable<T> && is_element_visitable<T>
static inline T recursive_transform(const T input, Fn func)
{
    T result = input;
    
    std::transform(input.begin(), input.end(), result.begin(), 
        [func](auto x) -> decltype(x) {
            return std::visit([_Func](auto&& arg) -> decltype(x) {
                return func(arg);
            }, x);
        }
    );

    return result;
}

There is no need to explicitly specify the type of x, at best it is the same as the type of argument it gets passed, at worst you make a mistake that compiles without errors but causes some subtle cast. And since you want to return a value that has the same type as x (so that we cast the result of func() back to a std::variant, just write -> decltype(x) as the trailing return type. You can do the same for the trailing return type of the lambda passed to std::visit().

Well, that would be true, except the above example is only so compact because you are copying by value, which leads me to:

Avoid unnecessary copies

I missed this in my previous review, but there are more places where you cause a copy to be made: anytime a function takes a parameter by value, it is copied. So to avoid the costly copies of large containers, be sure to pass the inputs as much as possible by const reference, both for the templated function parameters and for the parameters passed to the lambda functions.

Now we need a way to ensure the trailing return types don't become references. To do this, you can use std::remove_reference. It becomes a bit messier, so I would use a using declaration:

template<class T, class Fn> requires is_iterable<T> && is_element_visitable<T>
static inline T recursive_transform(const T &input, Fn func)
{
    using value_type = std::remove_reference<decltype(*input.begin())>::type;
    T result = input;
    
    std::transform(input.begin(), input.end(), result.begin(), 
        [func](const auto &x) -> value_type {
            return std::visit([_Func](auto&& arg) -> value_type {
                return func(arg);
            }, x);
        }
    );

    return result;
}

Remove redundant forward declarations

Every template definition is preceded by a forward declaration. In general, you should avoid unnecessary forward declarations, as it is repeating yourself and allows for accidental differences in the forward declaration and the actual definition. This is much more important for templates, because there the chance of the compiler noticing a conflict is much smaller.

Don't use long double unless you really need that extra precision

I see you use long double consistently in your code, but if you don't need the extra precision it might have over a double, that you probably pay the price in lower performance. The reason is that on x86 and x86_64, long double operations can only be done with x87 FPU registers and instructions, and not with SSE registers and instructions. There is also a large overhead storing long doubles.