1

I wrote a program that for every five seconds would print a random number (1-10) within a ten seconds timeframe. But it seems to be printing more than one random number every five seconds. Could anyone point me in the right direction?

clock_t start;

int random;

start = clock();

while (float(clock() - start) / CLOCKS_PER_SEC <= 10.0) {
    if (fmod(float(clock() - start) / CLOCKS_PER_SEC, 5) == 0 && (float(clock() - start) / CLOCKS_PER_SEC) != 0) {
        random = rand() % 10 + 1;
        cout << random << endl;
    }
}


return 0;
Karset
  • 5
  • 2
  • 3
    1) you never seed `rand` by calling `srand`. 2) you shouldn't be using `rand` in the first plase, but the modern [](https://en.cppreference.com/w/cpp/numeric/random) facilities (see also: [rand() Considered Harmful](https://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful)). 3) you are busy looping. Use a proper timer where you can sleep until the timeout expires. – Jesper Juhl Jul 25 '18 at 11:38

2 Answers2

6

EDIT: I felt this answer was incomplete, because it does not answer your actual question. The first part now explains why your approach fails, the second part is about how to solve your problem in a better way.

You are using clock() in a way, where you wait for a number of specific points in time. Due to the nature of clock() and the limited precision of float, your check basically is equivalent to saying: Are we in a window [x-eps, x+eps], where x is a multiple of 5 and eps is generally small and depends on the floating point type used and how big (clock() - start) is. A way to increase eps is to add a constant like 1e6 to (clock() - start). If floating point numbers were precise, that should not affect your logic, because 1e6 is a multiple of 5, but in fact it will do so drastically.

On a fast machine, that condition can be true multiple times every 5 seconds; on a slow machine it may not be true every time 5 seconds passed.

The correct way to implement it is below; but if you wanted to do it using a polling approach (like you do currently), you would have to increment start by 5 * CLOCKS_PER_SECOND in your if-block and change the condition to something like (clock() - start) / CLOCKS_PER_SECOND >= 5.


Apart from the clock()-specific issues that you have, I want to remind you that it measures CPU time or ticks and is hardly a reliable way to measure wall time. Fortunately, in modern C++, we have std::chrono:

auto t = std::chrono::steady_clock::now();
auto end = t + std::chrono::seconds( 10 );
while( t < end )
{
    t += std::chrono::seconds( 5 );
    std::this_thread::sleep_until( t );
    std::cout << ( rand() % 10 + 1 ) << std::endl;
}

I also highly recommend replacing rand() with the more modern tools in <random>, e.g.:

std::random_device rd; // Hopefully a good source of entropy; used for seeding.
std::default_random_engine gen( rd() ); // Faster pseudo-random source.
std::uniform_int_distribution<> dist( 1, 10 ); // Specify the kind of random stuff that you want.
int random = dist( gen ); // equivalent to rand() % 10 + 1.
Markus Mayr
  • 4,038
  • 1
  • 20
  • 42
5

Your code seems to be fast enough and your calculation precision small enough that you do multiple iterations before the number you are calculating changes. Thus, when the condition matches, it will match several times at once.

However, this is not a good way to do this, as you are making your computer work very hard. This way of waiting will put a rather severe load on one processor, potentially slowing down your computer, and definitely draining more power. If you're on a quad-core desktop it is not that bad, but for a laptop it's hell on batteries. Instead of asking your computer "is it time yet? is it time yet? is it time yet?" as fast as you can, trust that your computer knows how to wait, and use sleep, usleep, sleep_for, or whatever the library you're using is calling it now. See here for an example.

Amadan
  • 191,408
  • 23
  • 240
  • 301
  • I think your answer misses the point; the main factor that the condition is true multiple times is because `float` does not provide enough accuracy, not because the CPU performs multiple iterations of the loop per clock. – Markus Mayr Jul 26 '18 at 08:49
  • 1
    @MarkusMayr: You're right; that is what I intended to say but fumbled my words. +1 to you. – Amadan Jul 26 '18 at 09:03