2
\$\begingroup\$

Consider the following (C++20) function (from the solution to this StackOverflow question):

template<std::size_t N, class T>
constexpr auto array_repeat(T&& x)
{
    auto deref = []<std::size_t> (T x) { return x; };
    auto impl = [&]<std::size_t... Indices>(T&& x, std::index_sequence<Indices...>) 
    {
        return std::array{deref.template operator()<Indices>(x)...};
    };
    return impl(std::forward<T>(x), std::make_index_sequence<N>());
}

What this function does is take the argument x and repeat it N times in an std::array constructor.

Now, this works, but - I don't like the amount of hoops I have to jump through, I don't like having to nest two helper functions/lambdas, I don't like to use std::index_sequence's, and I certainly don't like to have to say "template operator" :-(

Is it possible to use fold expressions to generate the N repetitions more elegantly, or tersely, or both? Also, if you have other ideas for simplifying this code, that would be appreciated. I'm mostly interested in C++20 here, but C++23 or C++26 are also fine.

Note: You may not assume T is default-constructible.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ This works since C++17. template<std::size_t N, typename T> auto array_repeat(T const& t) { return std::apply([&](auto... e) {return std::array{(e, t)...};}, std::array<int, N>{});} \$\endgroup\$ Commented Sep 27 at 10:37
  • \$\begingroup\$ @AjinkyaKamat: That doesn't even compile. Also, you seem to be assuming that T is default-constructible, which is not necessarily the case. \$\endgroup\$ Commented Sep 27 at 13:20
  • \$\begingroup\$ You are wrong. Not only does it compile but also T can be non default-constructible. Check this out godbolt.org/z/1YqPv6s87 \$\endgroup\$ Commented Sep 27 at 14:13
  • 1
    \$\begingroup\$ @AjinkyaKamat: Yeah, I guess that does compile. I need to give that a longer look. Perhaps make it an answer? \$\endgroup\$ Commented Sep 27 at 15:00

2 Answers 2

5
\$\begingroup\$

Certainly it can be done more efficiently, by not constructing copies to pass through a lambda, adding a move or copy.
Just construct directly in the array using the comma operator.

It can also be done more generally, by allowing for case zero, which is important for generic code.
Yes, that means you must specify the template arguments for the std::array.

As a bonus, it is more concise and might even be more elegant.

#include <array>
#include <type_traits>
#include <utility>

template<std::size_t N, class T>
constexpr auto array_repeat(T&& value)
{
    return [&]<std::size_t... Indices>(std::index_sequence<Indices...>) {
        return std::array<std::decay_t<T>, N> { (void(Indices), value)... };
    } (std::make_index_sequence<N>());
}

I haven't changed the parameter from T&& to T const& to allow the rare T& copy constructors.

I haven't used the std::copy_constructible concept, because that requires more than needed here.

I added the necessary includes, which really shouldn't be omitted for a code review.

\$\endgroup\$
4
\$\begingroup\$

This function is missing the necessary definitions for it to be usable. I suggest adding

#include <array>                // also defines std::size_t
#include <utility>

The function should have a comment summarising its purpose. It doesn't need to be very extensive.

Accepting argument using forwarding reference suggests we might move from it, but we never do (the std::array aggregate construction always copies), so it's better to accept as const-ref.

Type T needs to be copy-constructible, so we could specify that as a constraint, helping users by giving more meaningful errors.

Whilst N is easily understood as a size (and familiar to std::array users), x isn't very meaningful as argument name.

We can also use comma operator instead of defining deref to simplify converting the index sequence entries to array values:

#include <array>
#include <concepts>
#include <utility>

// Construct an array of size N, with every
// element initialised with value.
template<std::size_t N, std::copy_constructible T>
constexpr auto array_repeat(T const& value)
{
    return [&value]<auto... Indices>(std::index_sequence<Indices...>)
    {
        return std::array{ ((void)Indices,value)... };
    }(std::make_index_sequence<N>());
}
\$\endgroup\$
6
  • \$\begingroup\$ I indeed omitted the inclusion of standard library headers, and a proper doxygen comment. In the project this function is used in, those do exist. I was just focusing on the part of the code which is relevant for my concern regarding syntactic/structural complexity. Otherwise, +1 for this approach, avoiding std::apply() - although I like defining lambdas separately from their use, so I would arrange it slightly differently. \$\endgroup\$ Commented Sep 28 at 8:50
  • \$\begingroup\$ So you haven't shown us the real code? Asking for reviews of example/hypothetical code is not how we do things here. \$\endgroup\$ Commented Sep 28 at 8:53
  • \$\begingroup\$ Of course I've shown you the real code. Just not all real code, which I can't (and shouldn't) share in a question focused on a particular function. \$\endgroup\$ Commented Sep 28 at 8:55
  • \$\begingroup\$ Context is important. If we have to guess at e.g. includes, then the reviews you get won't be as good as the otherwise will be. It's always a good idea to ensure that the functions you present are complete, even though they have been isolated from a bigger project. \$\endgroup\$ Commented Sep 28 at 8:57
  • \$\begingroup\$ This would presumably be used in templates. So, why fail zero? \$\endgroup\$ Commented Sep 28 at 14:34

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.