Problem with random numbers...

[Omega234]

Hello, everyone...

Lately I'm having trouble with my latest program - I'm trying to create a program for my friend (who needs it for her little brother, but that's not really so important), and the program is supposed to test a person on their vocabulary. The program reads in a list of values from a file named vocabulary.csv. So far, I'm only working on making it so that it give you a word, and you have to select the correct definition, but I'm having trouble getting the multiple choices to be random.

I want the program to give you the word, and then offer four options which are all different, but one of them is the answer. Right now, it tends to be that at least two of those choices are the same. Another problem is that the randomly-selected answer word seems to change throughout my code somehow (even though I don't even mention it in the code until I need it).

At the moment, the code only outputs the options and the "answer" and does not yet ask for options. Here's what the .csv file hold right now:

tricycle,a three-wheeled vehicle
cube,a hexahedron with six equal squares as faces
box,a (usually rectangular) container that may have a lid - used for storage
sphere,a three-dimensional closed surface such that every point on the surface is equidistant from the center

And here's my code so far (a bit lengthy, I know...):

#include <cstdio>
#include <stdlib.h>
#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
using namespace std;

struct word{
	string word;
	string definition;
} words[5], *iWords;

	// Variables
	int f = 0;
	int z = 0;

void printWord (word userWord)
{
	cout << "		 WORD: " << userWord.word << endl;
	cout << "	DEFINTION: " << userWord.definition << endl;
}

void fileLoader();
void wordTest();
void definitionTest();

int main()
{
	srand (time(NULL));
	system("CLS");
	if (f == 0)
	{
		fileLoader();
		main();
	}
	else if (f == 1)
	{
		cout << "*** MAIN MENU ***" << endl;
		cout << "Options:\n";
		cout << "1. Test words\n";
		cout << "Insert option: ";
		int userChoice;
		cin >> userChoice;
		if (userChoice == 1)
		{
			wordTest();
		}
		system("PAUSE");
		main();
		return 0;
	}
}

void fileLoader()
{
	ifstream values("vocabulary.csv");
	string temp;
	stringstream bar;
	
	if (values)
	{
		int i = 0;
		while (!values.eof())
		{
			getline(values, temp);
			bar << temp;
			while (bar.good())
			{
				getline(bar, temp, ',');
				words[i].word = temp;
				getline(bar, temp, ',');
				words[i].definition = temp;
			}
			bar.clear();
			i = i + 1;
		}
	}
	f = 1;
}

void wordTest()
{
	system("CLS");
	int rnd1 = rand() % 4 + 0;
	int m = rnd1;
	word answer = words[m];
	cout << "Which definition matches the word \"" << answer.word << "\"?" << endl << endl;
	word options[4];
	int test[4];
	int compare[4];
	int rnd2;
	int rnd3;
	for (int u = 4; u > 0; u--)
	{
		rnd2 = rand() % 4 + 0;
		for (int h = 4; h > 0; h--)
		{
			if (compare[h] == rnd2)
			{
				rnd3 = rand() % 4 + 0;
				compare[u] = rnd3;
				options[u] = words[rnd3];
			}
			else
			{
				compare[u] = rnd2;
				options[u] = words[rnd2];
			}
		}
	}
	cout << "Options:\n";
	for (int x = 4; x > 0; x--)
	{
		printWord(options[x]);
		cout << endl;
	}
	cout << "\nAnswer:\n";
	printWord(answer);
}

I know it's probably not the cleanest code, but I only started yesterday, so... any help would be greatly appreciated. Thanks!

[GCDEF]

I didn't get to your random problem yet, but never call main directly like that. The way you're going having the function repeatedly call itself is called recursion and unless you control it properly your program will get away from you and crash.

[Omega234]

Hmm... Is there another way to have it called repeatedly? I want the user to be able to input the beginning option multiple times (after going through the whole things once). To rephrase, I want to give them the options, have them select one, do whatever that option does, and then get to choose again.

So is there a better way to do that?

[GCDEF]

Use a while loop.

call fileloader in the beginning of main then something like

while(f != 1)
{
// do stuff in here
}

or whatever condition is appropriate to keep the program looping

[Omega234]

Ah, okay, that makes sense. Thanks.

But can anybody else help me with the problem about the randomness? I have no idea what to do (I'm not a C++ guru by any means), and any help is greatly appreciated.

[laserlight]

I didn't get to your random problem yet, but never call main directly like that. The way you're going having the function repeatedly call itself is called recursion and unless you control it properly your program will get away from you and crash.

Not only that, but the C++ standard explicitly forbids calling the global main function within a program.

But can anybody else help me with the problem about the randomness?

Yes, but before we do that, I suggest that you get rid of your global variables, making them local variables instead. Then you pass those variables that are needed to the various functions that are called. Post your resultant code, with the previously mentioned loop included.

[GCDEF]

Although it may not seem like it now, your looping is a bigger problem than your random numbers. Get it straightened out first. By call main repeatedly, you're calling srand repeatedly which may be the problem.

[Omega234]

Alright, so I've rewritten the main() function, and nothing in the program makes any calls to main(). I've also removed any global variables, and all variables are initialized locally.
I didn't know that it was bad to call the main() function more than once - I'll be sure to not do that in any of my codes from now on.

Here's the (complete) re-written code (only main() has really changed, but here's the whole thing):

/* This program is designed to take input from a CSV vocabulary list and test
the user on their knowledge of the words. */
#include <cstdio>
#include <stdlib.h>
#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
using namespace std;

struct word{
	string word;
	string definition;
} words[5], *iWords;

void printWord (word userWord)
{
	cout << "		 WORD: " << userWord.word << endl;
	cout << "	DEFINTION: " << userWord.definition << endl;
}

void fileLoader();
void wordTest();
void definitionTest();

int main()
{
	int f = 0;
	int z = 0;
	srand (time(NULL));
	system("CLS");
	fileLoader();
	while (z == 0)
	{
		system("CLS");
		cout << "*** MAIN MENU ***" << endl;
		cout << "Options:\n";
		cout << "0. Quit\n";
		cout << "1. Test words\n";
		cout << "Insert option: ";
		int userChoice;
		cin >> userChoice;
		if (userChoice == 0)
		{
			z = 1;
			break;
		}
		else if (userChoice == 1)
		{
			wordTest();
		}
		system("PAUSE");
	}
	return 0;
}

void fileLoader()
{
	ifstream values("vocabulary.csv");
	string temp;
	stringstream bar;
	
	if (values)
	{
		int i = 0;
		while (!values.eof())
		{
			getline(values, temp);
			bar << temp;
			while (bar.good())
			{
				getline(bar, temp, ',');
				words[i].word = temp;
				getline(bar, temp, ',');
				words[i].definition = temp;
			}
			bar.clear();
			i = i + 1;
		}
	}
}

void wordTest()
{
	system("CLS");
	int rnd1 = rand() % 4 + 0;
	int m = rnd1;
	word answer = words[m];
	cout << "Which definition matches the word \"" << answer.word << "\"?" << endl << endl;
	word options[4];
	int test[4];
	int compare[4];
	int rnd2;
	int rnd3;
	for (int u = 4; u > 0; u--)
	{
		rnd2 = rand() % 4 + 0;
		for (int h = 4; h > 0; h--)
		{
			if (compare[h] == rnd2)
			{
				rnd3 = rand() % 4 + 0;
				compare[u] = rnd3;
				options[u] = words[rnd3];
			}
			else
			{
				compare[u] = rnd2;
				options[u] = words[rnd2];
			}
		}
	}
	cout << "Options:\n";
	for (int x = 4; x > 0; x--)
	{
		printWord(options[x]);
		cout << endl;
	}
	cout << "\nAnswer:\n";
	printWord(answer);
}

However, my problem still exists with the random stuff...

It's ocurring somewhere in the wordTest() function. The word "answer" changes somewhere in the middle of it all, and I don't know why... add on to that the fact that the options tend to be the same. Again, any help is appreciated.

[Philip Nicoletti]

One problem is that you are accessing out of the range of the arrays.

you declare (amo)ng others

int compare[4];

but you are accessing compare[4].

The valid indices are 0,1,2,3 (not 4)

[GCDEF]

Next thing I notice (other than rand() % 4 + 0 - why +0?) is compare is used without being initialized, and as its size is 4, the maximum index you can use is 3. You're starting with 4. It's a little surprising it doesn't crash.

[Omega234]

Ah-ha! Well... that helps a bit.

So I've fixed my loops, and they all initalize at 3 and end when it's not greater than -1 (so it accesses 0, 1, 2, and 3). Now, the answer doesn't change at all, so this is definitely an improvement.

Here's the re-written wordTest() function:

void wordTest()
{
	system("CLS");
	int rnd1 = rand() % 4 + 0;
	int m = rnd1;
	word answer = words[m];
	cout << "Which definition matches the word \"" << answer.word << "\"?" << endl << endl;
	word options[4];
	int test[4];
	int compare[4];
	int rnd2;
	int rnd3;
	for (int u = 3; u > -1; u--)
	{
		rnd2 = rand() % 4 + 0;
		for (int h = 3; h > -1; h--)
		{
			if (compare[h] == rnd2)
			{
				rnd3 = rand() % 4 + 0;
				compare[u] = rnd3;
				options[u] = words[rnd3];
			}
			else
			{
				compare[u] = rnd2;
				options[u] = words[rnd2];
			}
		}
	}
	cout << "Options:\n";
	for (int x = 3; x > -1; x--)
	{
		printWord(options[x]);
		cout << endl;
	}
	cout << "\nAnswer:\n";
	printWord(answer);
}

My other problem still exists, though - many of the options come out the same. You say I should initialize compare - how? (I don't mean "how" literally - I mean in what way should I initialize it?) Or am I just missing something small in my function?

Oh, and you asked about my rand() % 4 + 0. I know that the +0 doesn't actually do anything, but it's for sake of ease of understanding it later. I have a terrible habbit of writing programs, taking a break from coding, and then coming back, and usually when I come back I forget why I did something a certain way. So instead of dealing with that, I just write all my rand()s with the + operator as well. I know it seems silly, but it doesn't do anything bad (right? Or is this another mistake I shouldn't be making?), so I don't mind it. I suppose I'd call it a "style" thing - I just sort of like it that way.

As usual, thanks for all the help so far (and any help I get from now on, too!).

[GCDEF]

I'm having trouble determining what you're trying to do. What is the purpose of compare?

Seems like maybe you have the cart before the horse. Are you sure your algorithm even makes sense? Perhaps you should write it out in pseudocode before you write actual C++.

[Omega234]

Well... it doesn't work, but compare was supposed to be used to make sure that the same word isn't chosen twice.

Pseudocode? I've never heard of this, but do you mean I should write the program in English first or something? And (not to sound stupid, but) how would you write that? Would you literally write the code in English, so "int x = 0" would be "Create an integer named "x" of value 0"?

Since I'm not sure exactly what you mean, I'll just go over how the function ought to work. It picks a random word from the possible words and this is the answer - what the user is supposed to be guessing. It then asks the user to pick an option.
In the loops, it's going for four times (because there are only four words in the array for now). It randomly creates a number between 0 and 3 and then it'll make options[iteration of loop] (i.e. options[1] or whatever) equal to words[random number]. The overall idea is to randomly pick a word.
The other loop is to make sure that the same word isn't picked twice (not that it's doing much good right now). The random word is checked against compare[iteration of loop]. If they're the same, create a new random number (set compare[iteration of loop] equal to it), and then set options[iteration of big loop] equal to words[random number]. The overall idea here is to make sure that the randomly picked word isn't chosen twice.
After all the checking and assigning, it outputs the possible options. For now, it also outputs the answer (because I'm testing the code - when I'm done, you won't see the answer displayed).

I realize that all the loops probably aren't the best idea (and seeing how things tend to go for me, it's probably a lot more complicated than it needs to be), so any help you can give would be great (or if there's an easier way, I'd like to know that too). Thanks for continuing to try to help (and thanks to anyone else who responds, too!)!

[GCDEF]

I guess it's me, but I'm still having a little trouble following along.

The first thing you need to do is initialize compare. I assume valid values in compare are 0 -3, and if you don't initialize it, they could occur there randomly. Set each element of compare to some other value such as -1.

Any particular reason you're writing your loops backwards? Usually one starts at 0 and works up. I don't see an advantage to counting down here.

If it were me and I found a random number had already been used, I'd just keep incrementing it until I found on that hadn't, remembering to wrap when you get to 3. rand() + 1 is still random.

Most importantly, it's time to learn to use the debugger, so you can step through your code line by line watching what it's doing and looking at the variables it's using to make its decisions.

[Omega234]

Alright, so I've initialized compare. All the values are -1.

I also rewrote my loops - no idea why I did that, with the backwards counting...

I see what you mean about the incrementing, but how would I implement that? The program is eventually going to be given many more values than 4 (I'm just using a small number for sake of ease), and wouldn't that take a while? Or would my way (below) take longer? Or does it even matter?

So... I've completely rewritten the loop section. I think the current one is easier to understand than the last one, but I'll take your input for that. Neither of my rewritten versions work - I still get repeats. I know why now, though - a random number between 0 and 3 can occur more than once. So how would I fix that? Here's the new code (with my older rewritten code in comments):

void wordTest()
{
	system("CLS");
	int rnd1 = rand() % 4 + 0;
	word answer = words[rnd1];
	cout << "Which definition matches the word \"" << answer.word << "\"?" << endl << endl;
	word options[4];
	int compare[4] = { -1, -1, -1, -1 };
	/*int rnd2;
	for (int u = 0; u < 4; u++)
	{
		rnd2 = rand() % 4 + 0;
		for (int n = 0; n < 4; n++)
		{
			while (compare[n] == rnd2)
			{
				rnd2 = rand() % 4 + 0;
			}
		}
		compare[u] = rnd2;
		options[u] = words[compare[u]];
	}*/
	for (int x = 0; x < 4; x++)
	{
		for (int y = 0; y < 4; y++)
		{
			if (compare[x] == compare[y])
			{
				if (x != y)
				{
					compare[x] = rand() % 4 + 0;
					break;
				}
			}
		}
	}
	compare[3] = rand() % 4 + 0;
	for (int x = 0; x < 4; x++)
	{
		options[x] = words[compare[x]];
	}
	cout << "Options:\n";
	for (int x = 0; x < 4; x++)
	{
		printWord(options[x]);
		cout << endl;
	}
	cout << "\nAnswer:\n";
	printWord(answer);
}

The new loop is designed to make sure that none of the values are repeated... it's obviously not working, though, because the values are declared after all that checking - how would I do it otherwise? Should I set all the random values first, and then go through and make sure none are repeated?

Thanks for all your help so far, but I just need a bit more (hopefully only a bit...). Thanks for all help!