3
\$\begingroup\$

I wrote this code for an answer over on SO. I suspect it's possible to clean it up a bit, particularly the base-case specialization for FactorialArray, which is repetitive and contains odd bits (for instance: why double curly braces? because clang complains about un-braced subobject initializers otherwise...)

Any and all suggestions are appreciated. Note that Factorial is separate from FactorialArray for pedagogical clarity; I realize this makes the compile-time calculation O(N2) unless the compiler is clever enough to memoize it.

#include <iostream>
#include <cinttypes>
#include <array>

using std::uint64_t;

// Helper template that computes the factorial of one integer
template<uint64_t I> struct Factorial
{ static constexpr uint64_t value = I * Factorial<I-1>::value; };

template<> struct Factorial<0> { static constexpr uint64_t value = 1; };

// FactorialArray recursively assembles the desired array as a variadic
// template argument pack from a series of invocations of Factorial
template<uint64_t I, uint64_t... Values> struct FactorialArray
  : FactorialArray<I-1, Factorial<I>::value, Values...>
{};

// and in the base case, initializes a std::array with that pack
template<uint64_t... Values> struct FactorialArray<uint64_t(-1), Values...>
  : std::array<uint64_t, sizeof...(Values)>
{
  constexpr FactorialArray()
    : std::array<uint64_t, sizeof...(Values)> ({{Values...}})
  {}
};

int main()
{
  static FactorialArray<10> f;
  for (std::size_t i = 0; i < f.size(); i++)
    std::cout << i << "! = " << f[i] << '\n';
  return 0;
}
\$\endgroup\$
7
\$\begingroup\$

I think your code is fine for the most part. I don't like the inheritance from std::array, seeing as you can do the same thing with a static array data member, which would make the code easier to maintain (less coupling).

If you're well-and-truly in C++14 land, I think the code is clearer if you use constexpr functions rather than functions-disguised-as-structs:

#include <iostream>
#include <utility>
#include <array>

constexpr size_t factorial(size_t t) {
    return t == 0 ? 1 : t * factorial(t - 1);
}

template<size_t... i>
constexpr auto factorial_array(std::index_sequence<i...>) {
    return std::array<size_t, sizeof...(i)>{{factorial(i)...}};
}

template<size_t size>
constexpr auto factorial_array() {
    return factorial_array(std::make_index_sequence<size>{});
}

int main() {
  constexpr auto f = factorial_array<15>();
  for (size_t i = 0; i < f.size(); i++) {
    std::cout << i << "! = " << f[i] << '\n';
  }
  return 0;
}
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I didn't know this was even possible. What parts of it require C++14 and not just C++11? \$\endgroup\$ – zwol Jul 23 '16 at 13:09
  • \$\begingroup\$ index_sequence isn't in the standard library until C++14, and you'd need to explicitly declare return types rather than using auto. \$\endgroup\$ – Reuben Thomas Jul 23 '16 at 13:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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