Header file causes problems with the original array after function call

Question

I created a header file called primes.h. What I want is to call a function generate_primes() defined in this header file to fill an array with a determined number of prime numbers.

So to test if everything works fine, I first printed the elements of the array from 'inside' the header file' which works just fine. Then I tried printing them from outside the header file, in the program including the header file but it's not working, and the program just stops after printing a few elements. Why is this unproper behaviour? And how can I do this the right way. Thanks for helping.

The header file:

int generate_primes(const unsigned long long n, unsigned long long*prime) 
{

    try
        {
            prime = new unsigned long long[n]; 
        } 
    catch(const std::bad_alloc&e) {
        std::cerr << e.what(); 
        exit(EXIT_FAILURE);  
    }

    prime[0] = 2, prime[1] = 3, prime[2] = 5; 

    unsigned long long p;
    unsigned long long index = 3;

    for (p = 7; index < n; p += 2)
        {
            short isPrime = 1; 
            unsigned long long test_limit = (unsigned long long) sqrt(p); 

            for (int i = 1; prime[i] <= test_limit && i < index; i++)
                {
                    if (!(p%prime[i]))
                        {
                            isPrime = 0; 
                            break;  
                        }   
                } 
            if (isPrime) prime[index++] = p; 
        }

    //everything works fine when I print from inside the header file 
    /*for (int i = 0; i < n; i++)
      {
      if (i && i % 7 == 0) std::cout << '\n'; 
      std::cout << prime[i] << "\t\t"; 
      }
    */
    return 1; 
}

My program:

#include <iostream>
#include <cmath>
#include <vector>
#include <new>
#include <cstdlib>
#include "primes.h"

int main(){
    unsigned long long n = 100;//example  
    unsigned long long *prime;   
    generate_primes(n, prime); 

    //but printing from here causes problems 
    //isn't the array now filled after calling the above function ? 
    for (unsigned long long i = 0; i < n; i++) 
        {
            if (i && i % 5 == 0) std::cout<< '\n'; 
            std::cout << prime[i] << "\t"; 
        } 

    return 0; 
}

Your problem is that the pointer parameter is only changed within the function but not reflected outside. You can change the parameter signature like this: unsigned long long*&prime to get your code working. — user0042
– user0042, Commented Jul 12, 2017 at 19:47

Barmar · Accepted Answer · 2017-07-12 19:48:19Z

2

The problem is that the parameter prime is being passed by value, not by reference. So when generate_prime assigns:

prime = new unsigned long long[n];

this only assigns to the local variable prime, not the caller's variable. You need to pass the parameter by reference so that the assignment will be visible to main().

int generate_primes(const unsigned long long n, unsigned long long*&prime)

answered Jul 12, 2017 at 19:48

Barmar

789k57 gold badges555 silver badges669 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

K. Kirsz Over a year ago

It would generally be better to just allocate the whole array at the caller's side and manage it's lifetime from there. Now it is unclear who and how should take care of the array. Then you wouldn't even have such issues.

Barmar Over a year ago

@K.Kirsz And it would be even better to use std::vector so this whole thing becomes moot.

user0042 Over a year ago

@K.Kirsz The best way would be to use a std::vector reference parameter.

Younes Over a year ago

Thank's. I just meant to have this part of code seperated and be able to use It whenever I need It.

tadman Over a year ago

@lamriniyounes There's a lot of valid criticisms here you should pay attention to. Code separation is just one problem.

|

tadman · Accepted Answer · 2017-07-12 19:44:27Z

0

That's not a header file, it's a bunch of code. If you want to separate that code it should be in another .cpp file.

An associated .h would look like this:

#include <cmath>
#include <vector>
#include <new>

int generate_primes(const unsigned long long n, unsigned long long*prime);

Then you'd have generate_primes.cpp or something like it:

 #include "generate_primes.h"

 int generate_primes(const unsigned long long n, unsigned long long*prime) {
   // (function body)
 }

To build your application you'd need to compile both .cpp files and link them together with the required libraries.

answered Jul 12, 2017 at 19:44

tadman

212k23 gold badges237 silver badges266 bronze badges

2 Comments

Younes Over a year ago

You're right, It's not. What I meant to do is to have this part of code seperated since I use It alot. Then be able to call It whenever I need It.

tadman Over a year ago

That's exactly why you break out your code into multiple .cpp files, and each .cpp with functions in it has an associated .h file that defines what those are. This complicates compilation, but you'll have to commit to a Makefile, or something like it eventually anyway. With a good compiler setup then only .cpp files which need to be recompiled will be compiled during a routine build, and this can speed up development considerably. the way you have it in the .h file leads to cascading rebuilds even if the function signature is untouched.

milbrandt · Accepted Answer · 2017-07-12 20:27:02Z

0

The value of the uninitialized pointer prime is an argument to the call of generate_primes. There it is not evaluated but the parameter assigned a new (local) value. Therefore inside the method the Array is accesible. But the new value is never returned, so in main the value of prime remains unchanged.

Additionally you should change n from unsingned long long to a smaller type for teh sake of the compiler.

Two possible solutions: a) modify generate_prime so that it return the address of the Array

unsigned long long *prime generate_primes(const unsigned long n)

b) gíve the address of the pointer as an Argument to the method

int generate_primes(const unsigned long n, unsigned long long **prime) 
{

    try
    {
        *prime = new unsigned long long[n]; 
    } catch (const std::bad_alloc &e) {
    // your code

and

int main(){
    unsigned long n = 100;//example  
    unsigned long long *prime;   
    generate_primes(n, &prime);

    // your Code

    delete[] prime; // free memeory
    return 0; 
}

edited Jul 12, 2017 at 20:27

answered Jul 12, 2017 at 20:10

milbrandt

1,4952 gold badges16 silver badges20 bronze badges

1 Comment

Younes Over a year ago

Thank's. I tried : int generate_primes(const unsigned long long n, unsigned long long*&prime) wich is the same.

Collectives™ on Stack Overflow

Header file causes problems with the original array after function call

3 Answers 3

7 Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

7 Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related