Prime Nnumbers and Genterating memory

[raynierl]

The program needs to get all the prime numbers from a random generated array of 50,000 integers, Take all the prime numbers out and put them in another array filled with memory generated from the heap, and prints them all out.

 #include<iostream>
#include<vector>
#include<ctime>
#include<cstdlib>
using namespace std;
int main()
{
	int primes =0;
	srand((time(NULL)));
	int size = 50000;
	int x[size];
	for (int i=0;i<size;i++)
	{
		x[i] = 1+rand() % (10-1)+1;
	if (x[i] % 2 != 0)
		primes++;
	}
	int prime[primes];
	int *a = new int[primes];
	for (int i =0;i<=primes;i++)
	{
		if (x[i] % 2 !=0 or x[i] == 2)
		prime[i] = x[i];
    }
		for (int i=0;i<primes;i++)
		{
			a[i] = prime[i];
		}
	cout << "Number of Primes: " << primes << endl;
		cout << "Array : ";
	for(int i =0; i<primes;i++){
	if ( i == primes-1)
	{cout <<a[i] << endl;}
	else
	cout << a[i] << " , ";
	}
	delete[] a;
	return 0;
}

The problem is the compiler generated either junk or 0 for some of the values(Don't know why, HELP)

[Chervil]

Rather strange algorithms in use.

First generate 50000 random numbers between 2 and 10 inclusive.
Count how many of them are odd (on average, it will be 4/9 * 50000).

Allocate two arrays of size roughly 22222.

For each of those 22,000ish elements, if the corresponding element in the first array is either odd, or equal to 2, copy the value, otherwise leave the element unchanged.

Copy the second array to the third.

Print out the contents of the third array.

By the way, variable length arrays are not a standard feature in C++. Array a[] is the only one correctly allocated and freed.
You might better use std::vector for all of the arrays.
instead of

    int x[size];

use

    vector<int> x(size);

There is also an out of range access because the loop

for (int i =0;i<=primes;i++)

repeated one time too many.

The garbage values are simply those where nothing was copied to the primes array.

[raynierl]

I see my error for :
for (int i =0;i<=primes;i++)
but why would making it a vector fix help?

[Chervil]

The background to my comment was that I prefer to recommend ISO standard C++, which doesn't permit variable-length arrays.

So int prime[primes]; isn't standard C++ if primes is a variable. You could use

    int * prime = new int[primes];

however

    vector<int> prime(primes);

does effectively the same job without any need to worry about later having to use delete.

The rest of my comments may not have been very helpful. The main point was that I don't think you correctly identify primes.

[raynierl]

I allocated prime to new memory of size primes, but i still have the same problem.

[Chervil]

Well, there are at least three things to consider
1. Did you actually intend to fill the buffer with the values in the range 2 to 10 (generated by 1+rand() % (10-1)+1 which is the same as 2+rand() % 9. I just felt it was a rather small range of values (but I don't know what you intended to do).

2. When counting how many primes there are, primes++; at line 16, the test on the previous line is not testing for primes, but for odd numbers. (9 is odd but it is not prime).

3. in the loop where values are copied from that first array to the primes arrayprime[i] = x[i]; there are two more problems.
3.a the test if (x[i] % 2 !=0 or x[i] == 2) is different to the one used previously, but it still doesn't correctly test for primes (it accepts all odd numbers and the number 2)
3.b The assignment statement should use two different subscriptsprime[k] = x[i];, Here k should start from zero, add 1 to it after each value is copied.

bool isprime(int n)
{
    // function to test if a number is prime
}

int main()
{
    int numprimes = 0;
    srand((time(NULL)));
    const int size = 50000;
    int x[size];
    for (int i=0; i<size; i++)
    {
        x[i] = 2 + rand() % 9;
        if (isprime(x[i]))
            numprimes++;
    }
    cout << "primes = " << numprimes << '\n';

    int * prime = new int [numprimes];

    int k = 0;
    for (int i = 0; i < size; i++)
    {
        if (isprime ( x[i] ) ) 
        {
            prime[k] = x[i];
            k++;
        }
    }
    cout << "k      = " << k      << '\n';
    
    for (int i=0; i<numprimes; i++)
    {
        cout << prime[i];
        
        if ( i == numprimes-1 )
            cout << endl;
        else
            cout << ", ";
    }
    
    delete[] prime;
}

[48T7M4Gy]

Why create another array? Though if you have to it's only a couple of extra lines to count the primes as they are detected, create a corresponding dynamic array then add them in a new loop which filters out the non-primes.

#include <iostream>

int main()
{
    const int size = 50000;
    int number[size];
    int num = 0;
    
    srand (time(NULL));

    for(int i = 0; i < size; i++)
    {
        num = 2 + rand() % 9;
        if ( num > 2 and ( num % 2 == 0 or num % 9 == 0) )
            number[i] = num + 1000;
        else
            number[i] = num;
        
        std::cout << number[i] << ", ";
    }

    return 0;
}

[48T7M4Gy]

#include <iostream>

int main()
{
    const int size = 50000;
    int number[size];
    int num = 0;
    int count = 0;
    
    srand (time_t(NULL));
    
    for(int i = 0; i < size; i++)
    {
        num = 2 + rand() % 9;
        if ( num > 2 and ( num % 2 == 0 or num % 9 == 0) )
            number[i] = num + 1000;
        else
        {
            number[i] = num;
            count++;
        }
    }
    
    int *primes = new int[count + 1];
    int count_primes = 0;
    
    for(int i = 0; i < count; i++)
    {
        if ( number[i] < 1000)
        {
            primes[count_primes] = number[i];
            count_primes++;
        }
    }
    
    for(int i = 0; i < count_primes; i++)
    {
        std::cout << primes[i] << ' ';
    }
    
    
    return 0;
}

[48T7M4Gy]

It's noteworthy that if there are more primes to be considered because the random number limit is set higher than 10, then a separate array of prime numbers up to the limit could be prepared using a sieve. This array would be used for primality checking of random numbers as they are generated and stored in the 50,000 long array.

[raynierl]

@kemort thanks for your help, your code worked. but that still does not fully answer my question, because my code was producing junk. some of the numbers that were being outputted were incredibly huge and i have yet to understand why.

[Chervil]

Raynierl wrote:

my code was producing junk. some of the numbers that were being outputted were incredibly huge and i have yet to understand why.

I explained it here:

Chervil wrote:

The garbage values are simply those where nothing was copied to the primes array.

and here

Chervil wrote:

3.b The assignment statement should use two different subscripts

    prime[k] = x[i];

Here k should start from zero, add 1 to it after each value is copied.

Basically, you allocated a large array. At the start it is completely filled with garbage values. Later, some of those values were replaced by sensible numbers, but others were left unchanged.

[48T7M4Gy]

@Raynieri

Sorry for not addressing your question about junk values. In your program just initialise the array so your line 18 becomes int prime[primes] = {0};