C++

Forum

 
Infinite Loop Problems

Mojet (5)
So I'm messing around trying to generate an array with no duplicate numbers.I realize that I am making this way more complicated than it needs to be and that there are other ways to accomplish this.

I seem to have created an infinite loop, somewhere in the do while statement maybe? Other than it not exiting the loops it seems to function how I wanted it to. Suggestions?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
 
#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int main(){
    int temp,size=6,p[size];
    int dup;

    srand(time(0));

    for (int c=0;c<=size;c++){
        do{
            dup=0;
            temp=rand()% size;
                for (int i=0;i<=size;i++){
                    if(temp==p[i]) dup++;
                }
            }while(dup!=0);
            p[c]=temp;
            cout<<p[c]<<endl;
        }
    cout<<"End of the line"<<endl;

    return 0;
}
Last edited on
MikeyBoy (5631)
In the first line of your do-loop block, you set dup to 0. You then don't change it anywhere in the rest of your code. So how can the condition (dup != 0) ever be true?

You define the array p to have 6 elements, but then you loop over seven values for the array indices c and i (0 to 6). When you try and access p[6], you're accessing memory past the end of the array, which will give you undefined behaviour.
Mojet (5)
On line 17 I increment dup. Sorry I sort of hid it!

 
 
if(temp==p[i]) dup++;


Well I sort of fixed it.... except now it runs fine up until I set size to more than 16 before it starts spitting out crazy again.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
int main(){
    int temp,size=16,p[size];
    int dup;

    srand(time(0));

    cout<<"count    #"<<endl;
    for (int c=0;c<=size-1;c++){
        do{
            dup=0;
            temp=rand()% size;
                for (int i=0;i<=size-1;i++){
                    if(temp==p[i]){
                        dup++;
                    }
                }
            }while(dup!=0);
            p[c]=temp;
            cout<<c<<"        "<<p[c]<<endl;
        }
    cout<<"End of the line"<<endl;

    return 0;
}
MikeyBoy (5631)
On line 17 I increment dup. Sorry I sort of hid it!

Apologies for missing that. Although I would add that with a more consitent indentation and bracing style, it would be easier for both you and others to read your code and see what's going on. For example, it would make sense for the end of your while loop to be aligned with the start of it.

Moving that dup++ to a new line certainly helps - and bonus points for enclosing it in braces :)

You've ignored my comments about the fact that you're writing past the end of your array. Both your for loops are looping from 0 to 16, which means that in their final iterations they're attempting to access p[16], which is the 17th element of the array. However, you've declared p to only have 16 elements, so on that final iteration, you're going past the end of the array.

This will result in undefined behaviour.
Last edited on
Icedoe (4)
I think your problem's line 17; a non-changing whileloop followed by p[c]=temp.

edit: haha; missed the dostatement. sry
Last edited on
MikeyBoy (5631)
I think your problem's line 17; a non-changing whileloop followed by p[c]=temp.

Um, that's the while statement at the end of a do-while loop.

Admittedly, the fact that it's not aligned with the "do" part of the loop makes it less obvious...
Last edited on
Mojet (5)
So then I need to decrease the size by 1 in the for loops?
MikeyBoy (5631)
Yes. Think about it logically - your index is starting at 0, and your condition says that the loop will continue for as long as it is equal to or less than 16. That means that it will loop 17 times. But your array is only 16 elements long.
Izrayl (1)
Your codes should still function good or well.
Because the do while loop is to ensure each member of the array is assigned a different or unique number from the randomly generated numbers from 0 to size-1. Even if you change the value of size to any number above 16, the program should run.
cool stuff there.
Mojet (5)
In the for loops up above, is that not a good way to do it then?
condor (271)
Slightly modified to read it easy here's the code and the output working for size = 30.
I'm working on Linux and my command line for building the program is:

g++ -std=c++11 -Wall -o randarr randarr.cpp

and the compiler is gcc 4.8.2

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
 
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <iomanip>

using namespace std;

int main()
{
    int temp, size = 30, p[size];
    int dup;
    srand(time(0));

    for (int c = 0; c < size; ++c)
    {
        do
        {
            dup=0;
            temp=rand() % size;
	    for(int i = 0; i < size; ++i)
   	         if(p[i] == temp) 
                    ++dup;
        } while(dup != 0);
        p[c] = temp;
    }
    
    for(int i = 0; i < size; ++i)
		cout << setw(4) << i << setw(7) << p[i] << '\n';
	cout << '\n';
    return 0;
}
 
   0     13
   1     18
   2     16
   3     29
   4     10
   5      4
   6     26
   7     20
   8     14
   9      5
  10      3
  11     27
  12      9
  13     25
  14     28
  15      8
  16     21
  17     11
  18      7
  19     24
  20     15
  21     12
  22      1
  23     23
  24     19
  25     17
  26      2
  27     22
  28      0
  29      6
Mojet (5)
I got it now. Thanks for the helping me understand!
condor (271)
@Mojet

OK! You're welcome! Please, if you consider all OK, mark this thread as Solved.
condor (271)
Here's a dynamic approach of the problem(the memory size is unknown at the compilation stage). It's a flexible way to allocate only the needed RAM.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
 
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <iomanip>

using namespace std;

int main()
{
	int size;
	cout << "How many numbers? ";
	cin >> size;
	if(size <= 0) return -1;
	
	int *p = new int[size];		// dynamic memory allocation
	
        srand(time(0));
	
	int contor = 0;
	while(contor < size)
	{
	   bool flag = false;		// a search starts with flag down
	   int temp = rand() % size;
	   for(int i = 0; i < contor; ++i)
	      if (temp == p[i])
		{
		   flag = true;     	// was found a duplicate among p[]...
		   break;		// breaking the for loop
		}
	   if(!flag)                    // if the flag remains down then temp is good
              p[contor++] = temp;	// and is enlisted!
	}
    
        for(int i = 0; i < size; ++i)
           cout << setw(4) << i << setw(7) << p[i] << '\n';
	cout << '\n';
	delete[] p;		        // memory allocation release
       return 0;
}
 
How many numbers? 36
   0      4
   1      3
   2     34
   3     17
   4      0
   5     31
   6     14
   7     16
   8     24
   9     23
  10      2
  11     27
  12      6
  13     11
  14     12
  15     15
  16     10
  17     26
  18     28
  19     21
  20     25
  21      7
  22     35
  23      1
  24     19
  25     22
  26     32
  27      8
  28     29
  29     18
  30      5
  31     33
  32     30
  33      9
  34     20
  35     13

Try it and please ask for help.
Last edited on
Topic archived. No new replies allowed.