Tips for my beginner program in c++

[aidan8181]

Hi this is my first post, I'm new to programming and was wondering if I could get some tips on a program I have designed for a class I'm taking, I have completed the task and my program outputs what was expected from me. However I just really would like some pointers on how I could improve my code, I have been reading the forum that using "system" for example is a bad idea, this was explained to me in my class also but for this task I was told to use it. One thing I would really love to figure out is the for loops, as far as I can tell the first for loop is calculating the rows and the nested for loop is calculating the columns however I'm not 100% sure about this. Also if anyone spots anything else I could improve in the code and explain that to me that would be great thanks.

The program is to print out a random selection of stars on the screen while blinking, blinking and size of the screen and determined by the input of the user

#include <iostream>
#include <Windows.h>
using namespace std;

int main() {
	int userNum, blinkNum, i, j, v, h, x;

	cout << "Please enter how large you would like the night sky: ";
	cin >> userNum;
	cout << endl;

	cout << "Please enter how many times you would like the sky to blink: ";
	cin >> blinkNum;
	cout << endl;

	x = 0;
	system("CLS");

	do {
		for (i = 0; i < userNum; i++) {
			for (j = 0; j < userNum; j++) {
				v = rand() & userNum;
				h = rand() & userNum;
				if (i == v || j == h)
					cout << "*";
				else
					cout << " ";
			}
			cout << endl;
		}
		Sleep(500);
		system("CLS");
		x++;
	} while (x < blinkNum);

	return 0;
}

[LB]

- using namespace std; is bad practice - Google will tell you why.
- You should not declare a variable until you intend to use it, and you should avoid allowing a variable to exist in an uninitialized state. On line 6 you declare several variables that don't get used until much later in the program.
- You should avoid using rand() and the likes because it is being deprecated and may not be in a future standard of C++. Instead, use the new C++11 <random> header - there are examples of how to use it at http://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution#Example and similar.
- If you must use rand() remember that you need to call srand once at the start of your program - otherwise you will always get the same random sequence.

[dhayden]

On lines 22 and 23, I suspect that you want % instead of &. A&B does a bit-by-bit binary AND on the numbers A and B, and returns the result. A%B returns the remainder of dividing A/B.

[aidan8181]

hey folks thanks for the tips and the extra references, its great hear these things I'll have another go at the program and see what I can come up with.

Ok so I taken your advise LB and this is the program I have come up with:

#include <iostream>
#include <Windows.h>
#include <random>

int main() {
	int userNum;
	std::cout << "Please enter how large you would like the night sky: ";
	std::cin >> userNum;
	std::cout << std::endl;

	int blinkNum;
	std::cout << "Please enter how many times you would like the sky to blink: ";
	std::cin >> blinkNum;
	std::cout << std::endl;
		
	system("CLS");

	int i, j, x;
	x = 0;
	do {
		for (i = 0; i < userNum; i++) {
			for (j = 0; j < userNum; j++) {
				int v, h;
				std::random_device rdNum;
				std::mt19937 genNum(rdNum());
				std::uniform_int_distribution<> dis(0, userNum);
				v = dis(genNum);
				h = dis(genNum);
				if (i == v || j == h)
					std::cout << "*";
				else
					std::cout << " ";
			}
			std::cout << std::endl;
		}
		Sleep(500);
		system("CLS");
		x++;
	} while (x < blinkNum);

	return 0;
}

Again any more tips would be a great help.

The <random> header is quite useful as I could see myself using this kind of code again. What I have done is created a random device called rdNum and then a generator called genNum which using rdNum to generate random numbers and then distributor to create a random number between 0 and userNum. I hope thats what is going on as its seems a bit advanced for me could someone tell me if thats correct or not?