5
\$\begingroup\$

This question is a direct follow-up of:

Have a look at this Mersenne Twister random-number generator function


Thanks to Code Review and Stack Overflow, I now realize, that:

  • unsigned long long int is not guaranteed to be 64-bit

  • I don't want to re-initialize the random_device on every function call

and a few other things.


So, based on current information, I've re-written it as follows:

#include <iostream>     // std::cout
#include <cstdint>      // std::uint64_t
#include <random>       // std::random_device, etc.

std::uint64_t random_integer(std::uint64_t rand_min, std::uint64_t rand_max)
{
    // use Mersenne Twister as random-number generator engine
    // initialize = seed the random device; static thread_local = only once
    // avoid "most vexing parse" by using "uniform initialization syntax" = { something{}() }
    static thread_local std::mt19937_64 random_number_generator{std::random_device{}()};

    // uniform number distribution, guaranteed unbiased
    std::uniform_int_distribution<std::uint64_t> number_distribution(rand_min, rand_max);

    // return one random number
    return number_distribution(random_number_generator);
}

// this is just to test the output
int main()
{
    for (int i = 1; i <= 10; i++)
    {
        std::cout << random_integer(10000000, 100000000) << std::endl;
    }
    return 0;
}

If you could guide me further on improving it, I would be grateful.

\$\endgroup\$
6
\$\begingroup\$

Don't pay for what you don't use

I can imagine many situations where using the generator from one and only one thread would be quite reasonable. In that case, users would pay on each call to the function. It is one of the reasons why I prefer objects in these cases: it gives gun to hands of the users. Whether they shoot their foot or what they want to shoot is not my business any more. Though it is not excuse to make "easy to use incorrectly" interfaces.

C with classes interface

I'm not really sure why, but the exact kind of interfaces are prevalent. May be it is because I learnt modern C++ before learning anything else. Modern ways of doing things should come to mind first, and if they are not good fit, start searching for something else.

Modern (almost?) interface

Lets reconsider the interface (given you figured out how to share state, which leads to object):

template <typename ForwardIterator>
void generate_range(ForwardIterator first, ForwardIterator last)
{
    //state sharing mechanism

    while (first != last)
    {
        *first++ = number_distribution(random_number_generator);
    }
}

This enables following syntax:

std::vector<int> v(20);
generate_range(v.begin(), v.end());

But this requires resizing. Some people even might want to print 10 integers into something, like in your example. Lets handle that too:

template <typename OutputIterator>
void generate_some(OutputIterator d_first, std::size_t n)
{
    for (std::size_t i = 0; i < n; ++i)
    {
        *d_first++ = number_distribution(random_number_generator);
    }
}

This enable following:

generate_some(std::ostream_iterator<int>{std::cout}, 10);

Or, if you want to append into vector:

std::vector<int> v;
generate_some(std::back_inserter(v), 10);

And, just for symmetrical and intuitive interface:

generate_one()
{
    return number_distribution(random_number_generator);
}

Of course in all of the examples you'll get narrowing conversion warning, I was just too lazy to type out std::uint64_t (I don't have IDE autocomplete here). Also, maybe putting static_assert to check iterator types will output much better compilation error message.

User experience

Yes, yes, user experience. It is not only about graphical interfaces. What experience do you want your users to have? What use cases do they have? Usually you have a problem, then solve it. Creating an interface without use cases in mind is like solving non-existent problem.

I would design an interface using this algorithm:

  1. Write usages out in declarative form

  2. Write out some candidates

  3. If list is empty, make usages more concrete, go back to 1

  4. Narrow/order list based on "easy to use correctly, hard to use incorrectly"

  5. Order the list based on implementation complexity

  6. Start implementing from first to last, until the first working version

  7. Think if they'll ever need more control, stop if not.

  8. Port/factor out core functionality which has greatest amount of control, and base the interface on top of it. Usually template parameters with default arguments, function with default arguments, using declarations help. I've yet to see the cases where it would be impossible.

\$\endgroup\$
  • 1
    \$\begingroup\$ @Vlastimil, this one will be much harder to make compile. I recommend dipping your toes in templates and classes first, before assembling my suggestion into one piece of code. \$\endgroup\$ – Incomputable Nov 12 '17 at 14:33
0
\$\begingroup\$

It's almost never necessary to use std::endl. Just use "\n" instead. std::endl forces a flush of the buffer, costing time. The buffer flush will almost always happen automatically anyway (it will happen at least when the program exits)

\$\endgroup\$

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.