3
\$\begingroup\$

I've been away from C++ for some time and I'd like to get back up to speed with modern practices, so I implemented a small random number generator described here.

Any advice on this piece of code?

    #include <cstdint>
    #include <cstddef>
    #include <limits>
    #include <random>

// implementation of Bob Jenkins' small prng https://burtleburtle.net/bob/rand/smallprng.html
namespace Random {
    namespace detail { 
        using _rand32_underlying = uint32_t;
        using _rand64_underlying = uint64_t;

        template<size_t N> struct rand_type { using type = void; };
        template<> struct rand_type<32> { using type = _rand32_underlying; };
        template<> struct rand_type<64> { using type = _rand64_underlying; };

    }

    // public
    template <size_t N> 
    using rand_t = typename detail::rand_type<N>::type;
    using rand32_t = rand_t<32>;
    using rand64_t = rand_t<64>;

    template <size_t N>
    inline rand_t<N> rot(rand_t<N> x, rand_t<N> k) noexcept { return ((x << k) | (x >> (N - k))); }

    template<size_t N>
    class SmallPrng
    {
        public:
            using result_type = rand_t<N>;
            inline rand_t<N> min() { return std::numeric_limits<result_type>::min(); }
            inline rand_t<N> max() { return std::numeric_limits<result_type>::max(); }

            rand_t<N> a, b, c, d;

            inline rand32_t prng32() 
            {
                rand32_t e = a - rot<N>(b, 27);
                a = b ^ rot<N>(c, 17);
                b = c + d;
                c = d + e;
                d = e + a;
                return d;        
            }

            inline rand64_t prng64()
            {
                rand64_t e = a - rot<N>(b, 7); 
                a = b ^ rot<N>(c, 13);
                b = c + rot<N>(d, 37);
                c = d + e;
                d = e + a;
                return d;
            }

        public:
            explicit SmallPrng(result_type seed = 0xdeadbeef) noexcept
            {
                static_assert(!(N != 32 && N != 64), "You can only construct a small prng in 32 or 64 bit mode.");
                a = 0xf1ea5eed;
                b = c = d = seed;
                for(size_t i = 0; i < 20; ++i)
                    (*this)();
            }

            explicit SmallPrng(std::random_device &rd) : SmallPrng(rd()) {}

            inline rand_t<N> operator()() noexcept
            {
                if constexpr(N == 32) 
                    return prng32();
                return prng64();
            }
    };
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Shouldn't a, b, c, d be private? \$\endgroup\$ – Cris Luengo Nov 9 '18 at 23:18
  • 3
    \$\begingroup\$ Are you familiar with DeMorgan's Law? An expression like !(N != 32 && N != 64) is a bit silly. \$\endgroup\$ – Mike Borkland Nov 10 '18 at 0:29
4
\$\begingroup\$
  • The SmallPrng(std::random_device &rd) constructor seems like an utility method. It spares the client a single call to seed = rd(), but forces an inclusion of (otherwise unnecessary) <random>. IMHO, this constructor is absolutely unnecessary.

  • The purpose of the

            for(size_t i = 0; i < 20; ++i)
                (*this)();
    

    loop in the constructor is seems useless; it amounts to passing a different seed. In any case, use {} around the loop body.

  • An asymmetry between

            b = c + d;
    

    in prng32() and

            b = c + rot<N>(d, 37);
    

    in prng64() is striking. Needs a comment, at least.

  • As mentioned in comments, the static_assert(!(N != 32 && N != 64), ....) is equivalent to static_assert((N == 32 || N == 64), ....) which is IMHO much cleaner.

\$\endgroup\$
  • \$\begingroup\$ Thanks, I accepted your answer. However I think the loop in the constructor is not useless, it correctly initializes the whole state (a, b, c, d). \$\endgroup\$ – Bogdan B Nov 10 '18 at 10:20

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.