Can someone tell me if this is incorrect?

[CodeDragon25]

I've been at this issue for a bit: Trying to make a endless loop that outputs pseudo-random numbers. I'm using Visual Studio 2017, if that makes any difference.

#include "pch.h"
#include <iostream>
#include <random>
#include <stdlib.h>
#include <time.h>

using namespace std;

int timer() {
	int I = 60;
	
	while (I > 0){
		--(I);
	}
		if (I == 0) {
			I + 60;
		}
}

int main() {
	int rand();
		srand(rand() % 10 + 1);

		cout << rand();
}

/*I wanted to make a countdown timer for the number generation.
I thought that using a simple math loop could help, though if
there's a more elegant solution to the code, by all means let me know.

[int timer()]: Its supposed to subtract 1 from 60, and check whether or not
the difference is 0. When it is, it will (hopefully) add 60, and start over.

[int main()]: Its supposed to generate a pseudo-random number (excluding 0)...
Its also supposed to print the value that's been generated... I hope that
someone is able to help me out here... I just want to get the hang of
endless looping RNG programming down, so then I can move on to using it
somewhere else (like a game or something...)*/

[Handy Andy]

Hello CodeDragon25,

I have yet to test the changes I propose, but this is what I see.

Line 10. What is "I"? It would be easier with a better name.

Line 13. It is "--I" or "I--" with no ()s.

Line 16. You are adding 60 to "I", but not changing the value. What you want is "I += 60. This adds 60 to "I" and stores the result in "I".

Line 21. Is not the way to call or setup "rand()".

Lines 21 and 22 are reversed. I should be:

srand(static_cast<size_t>(time(NULL)));

rand() % 10 + 1;

That is what I see for now. I will load it up and test it then let you know what I find.

Hope that helps,

Andy

[Cheddar99]

I don't entirely understand what you're trying to do, as you don't call the timer function, and it doesn't seem to do anything anyway, except count from 60 to zero and then exit.

Also, you have the return type of timer defined as int, but you don't return anything.

You also test to see if the variable I is 0 (line 15), yet it must be zero otherwise you wouldn't have gotten past the while loop.

As mentioned above, you have a little error in your code, as you presumably want to add on 60 to I once you know it's zero, but then you'll exit the function anyway, so it really makes no difference whether you add on 60 or not, and next time into the function you're going to set I to 60 again.

In essence, it would seem you could write your timer function as below, and it would have the same effect (i.e. it'll take just a fraction of a second to execute and really does nothing):

void timer() {
    int i = 60;
    while (i > 0)--i;
}

The main function, with the random number generation, might be better written as per Andy's comment, or such as below:

int main(){
    srand (time(NULL));
    int random = rand() % 10 + 1;
    cout << random << endl;
}

Sorry if I'm misunderstanding what you're doing, but I'm sure you'll get all the help you need on here. Good luck.

[Handy Andy]

Hello CodeDragon25,

Cheddar99 has touched on the points that I found when I compiled the code.

This may not be quite what you want, but it works:

#include <iostream>
//#include <random>  // <--- Not needed for what you have.
//#include <stdlib.h>  // <--- Usually come through "iostream". should use "cstdlib"
#include <ctime>

//using namespace std;  // <--- Best not to use.

void timer()
{
	int I = 60;

	while (I > 0)
		--I;

	if (I == 0)
		I += 60;
}

int main()
{
	srand(static_cast<size_t>(time(NULL)));  // <--- Only needs done once.
	
	while (true)
	{

		//rand() % 10 + 1;  // <--- should be int randNum = rand() % 10 + 1;

		std::cout << ' ' << rand() % 10 + 1 << '\n';  // <--- Just as easy to do the call to "rand" here. Otherwise use the variable from the above line.

		timer();

	}
}

You will have to add the #include "pch.h"

The if statement in "timer()" although properly written has no real use as Cheddar99 says each time you enter the function "I" is set to 60 and when the function ends "I" is lost as it is a local variable.

"main" shows two way you could use "rand".

If you idea is to have "timer" be a wait time then the function would work better as:

void timer()
{
	std::this_thread::sleep_for(std::chrono::seconds(2));

	// Or
	//std::this_thread::sleep_for(std::chrono::milliseconds(1000));  // <--- Where 1000 equals 1 second. and 1500 would be 1.5 seconds.
}

But you could just as easily put this in "main" where "timer" is called.

If I have misunderstood what you want let me know. I will look at your original comments again and see if I missed something.

Hope that helps,

Andy

Edit: forgot to mention to notice the header files at the top.

[dutch]

HandyAndy wrote:

//#include <stdlib.h> // <--- Usually come through "iostream"

It seems as if you are saying that <cstdlib> is included when <iostream> is included. I haven't heard that before. Could you give a reference for that?

And it is incorrect to cast time()'s return value to a size_t, since srand takes an unsigned int, not an unsigned long (which is often what size_t is defined as). It's generally pointless to cast time's return value at all for this purpose, IMO. And shouldn't you say nullptr instead of NULL?

[Handy Andy]

@dutch,

I found with my versions of VS that "iostream" will include the file "ostream" which includes "stdlib.h" and so far with VS I have not had any problem with not including "stdlib.h" in a program. But as someone once said if you think you need it include it. I only said that based on OP using VS2017 and what I have found looking at the header files for "iostream" and what it includes.

On my IDE "size_t" is an "unsigned int". Maybe there is a difference in between "X64" and "X86" that I do not know about. You could always change "size_t" to "unsigned int" or "unsigned". I use "size_t" because it is less typing. I did not know about any differences.

Acording to http://www.cplusplus.com/reference/cstdlib/srand/?kw=srand void srand (unsigned int seed); This is what I have been working with so far and just trying to keep up with what is needed for the parameter.

I am coming to understand that you are correct with the parameter to "time" being "nullptr". In the past I have only seen either zero or "NULL" for the parameter. I will have to change this for the future. Thank you for the input.

Andy

[dutch]

@HandyAndy,

That's interesting. It's disappointing that the implementation can bring in headers that you don't specifically request. All the more reason not to use using namespace std, I guess, since you can't be sure what you'll get!

I looked into gcc's iostream and it includes a lot of other headers (within headers within headers), and apparently cstdlib is in there somewhere but I wasn't able to actually find it. But srand(time(nullptr)) compiles with no warnings when just iostream is included, so it includes ctime, too. And apparently they are included as stdlib.h and time.h since std:: is not needed in front of them. That's sad, really.

I agree with you that if you need it, include it. Leaving out a header is taking advantage of implementation details and can't be considered portable, or even to work with future versions of the same compiler.

[Grime]

dutch that being said, as long as you don't write "using namespace std" what headerfiles are included doesn't really matter because it takes hardly any time to include those files. Maybe the implementers thought it was necessary to include those headerfiles (contrary to including just what's needed, maybe this is for readability who knows), I mean we're talking about top programmers in the world here..

If you want a headerfile then explicitly include it. Thaz it. Headerguards will take care of making sure something is not included twice.

VS2017 has a thousand headerfiles within headerfiles which eventually lead to macro definitions. The headerfile you include need to make use of all those macro definitions, so if it really was about performance headerfiles wouldn't include other headerfiles. But I guess it comes down to readability and maintenance?

[dutch]

Grime wrote:

as long as you don't write "using namespace std" what headerfiles are included doesn't really matter

If only that were true, but as I said some headers are included in such a way that unrequested identifiers are exposed in the global namespace. For gcc, this compiles:

#include <iostream>
int main() {
    srand(time(nullptr));
    std::cout << rand() << '\n';
}

[Grime]

Those identifiers would be declared in global namespace even if you included their native headerfiles (correct me if I'm wrong).

But I completely agree with you. I wonder why they haven't put those under std::. There's gotta be a reason.

Probably most if not all C++ compilers include other headerfiles which could have declarations in global namespace. So I guess if you're gonna use the global space then you're gonna have to use a namespace (*shrug*)

But if you're really that annoyed, edit the headerfiles yourself. Simple ;p.
If the compiler will verify the headerfiles (which it probably won't, but you should do this anyway to preserve the original headers) then simply copy all files, with an extra few letters at the beginning maybe, to a new file and add the new file to include directory Voila.

[CodeDragon25]

I thank you all for your feedback and suggestions! I'm relatively new to C++, and I don't have much of an understanding as to how some of the timer functions work, so it makes me feel good to know that this is a good starting place to look for answers to my inquiries.

EDIT: I have followed the advice, but now I've run into the issue that it spits out the same number an indefinite amount of time before repeating this with another number... (opted for the for(;;){} loop, instead of timer.) any way to make it spit a different number out for every recurrence? (I know that the mentioned rand() % 10 + 1 tends to pick lower numbers more than higher ones, according to the reference entry on the rand function)

[dutch]

@Grime, My understanding is that if you include them as <cstdlib> and <ctime> then they should be under std, whereas if you instead say <stdlib.h> and <time.h> then they are in the global namespace. However, if I remember correctly, they are allowed to be global as well in the first case, so it's kind of messed up. To be safe, I guess the C library identifiers should generally be considered as unusable for other purposes.

And I'm not editing the headers! Who knows what I would break?! I was looking through them a couple of hours ago and they are ferociously complicated.

EDIT: Found this in the standard: http://eel.is/c++draft/headers#4

[JLBorges]

> they are allowed to be global as well in the first case,

Yes. And they are allowed to be within namespace std in the second case.

It is unspecified whether these names (including any overloads ...) are first declared within the global namespace scope and are then injected into namespace std by explicit using-declarations
http://eel.is/c++draft/headers#4

It is unspecified whether these names are first declared or defined within namespace scope of the namespace std and are then injected into the global namespace scope by explicit using-declarations.
http://eel.is/c++draft/depr.c.headers.other#1

Example:

The header <cstdlib> assuredly provides its declarations and definitions within the namespace std. It may also provide these names within the global namespace. The header <stdlib.h> assuredly provides the same declarations and definitions within the global namespace, much as in the C Standard. It may also provide these names within the namespace std.
http://eel.is/c++draft/depr.c.headers.other#2

[dutch]

@JLBorges, but why? Was it just a common practice at the time of standardization? Or is there some obscure "good reason" for this?

[JLBorges]

I think the writers of compilers and libraries would have insisted on this flexibility.

Almost all of them need to maintain both C and C++ library implementations, and would want to have a common code base for common functionality.

In many cases, the operating system libraries which they need to use for their implementation (for example Unix libc or Linux glibc) would already have had C library names in the global namespace.

[Handy Andy]

Hello CodeDragon25,

I did not mean for this to turn into an off topic discussion, but it has been useful nun the less.

At this point it would help if you post your latest code and maybe a different explanation of what you want the "timer" function to do.

[int timer()]: Its supposed to subtract 1 from 60, and check whether or not
the difference is 0. When it is, it will (hopefully) add 60, and start over.

Well the while loop does its job, but it is doing it so fast that you do not see any delay. Subtracting 1 from 60 until you reach zero happens in a matter of milliseconds not seconds. Subtracting 1 from 60 in a loop may appear to be what you want except that each iteration of the loop happens in milliseconds not a full second.

To show any delay I had to change "I" to a long and give it a value of 60,000,000, (exclude the commas), and even that delay is less than a second while it counts down to zero.

Given the line of code std::this_thread::sleep_for(std::chrono::seconds(2)); there is only two parts you need to know about. "seconds" can be changed to "milliseconds" and, mostly, the number in () is what you change for the amount of delay. I once had a teacher say that you do not have to know everything about a piece of code or a function just how to use it.

Post your latest code and let us see what it is doing.

Hope that helps,

Andy

[CodeDragon25]

Ah, yes... I have this currently:

#include "pch.h"
#include <iostream>
#include <ctime>
int main() {
for (;;) {

srand((unsigned)time(nullptr));
int RNG = (rand() % 10 + 1);

std::cout << RNG << " " << RNG<< " " << RNG << " " << RNG << " " << RNG << "\n";
}
}




It spits numbers, and (as it is a blemish, be as it may, ) I've opted for the endless loop method here. It spits out a number from 1-10, but it spits out long strings of one digit before switching to a different digit within the range... I was thinking that I could possibly set an array (If that might help?) and have it pull from there... Or possibly find some way to set a seed that is a loop it self? I think I might be going a bit too far with that last thought, but who knows? It might be crazy enough to work!

[Ganado]

You generally only want to seed your random number generator one time.
Put srand((unsigned)time(nullptr)); once, at the beginning of your program.

int RNG = (rand() % 10 + 1);

std::cout << RNG << " " << RNG<< " " << RNG << " " << RNG << " " << RNG << "\n";

Your variable, called RNG, is a variable. A variable only changes its value from being re-assigned. The function itself is rand().
If you want each number in a line to be random, you need to call rand() multiple times.

Right now, you have what's essentially equivalent to:

int a = 3;
std::cout << a << " " << a << " " << a << " " << a << " " << a << "\n";

Perhaps,

// Example program
#include <iostream> // cout
#include <cstdlib> // srand(), rand()
#include <ctime>  // time()

int main() {
    srand((unsigned)time(nullptr));

    for (;;)
    {
        for (int i = 0; i < 5; i++)
        {
            int r = rand() % 10 + 1;
            std::cout << r << " ";
        }
        std::cout << '\n';
    }
}

2 4 9 9 9 
4 7 4 1 10 
1 2 6 8 3 
6 3 3 3 5 
10 6 9 10 3 
3 2 5 9 6 
7 10 10 5 1 
10 1 9 5 3 
8 6 7 6 5 
1 1 7 3 6
... 

[CodeDragon25]

Thank you, all! The help is much appreciated, and I now have a random number generator program, that does what I usually think of for this sort of function!