-7

Can someone please tell, what is wrong with this code? The purpose is to find the factors from a given number 'n'.

vector <int> * factor(int *n){
    vector<int> * arr;
    for(int i=1;i<=*n;i++){
        if(*n%i==0){
            arr->push_back(i);
        }
    }
    return arr;
}
int main(){
    int n,j=0;
    vector<int> *arr;
    cin>>n;
    arr = factor(&n);
    for(auto it=arr->begin();it!=arr->end();++it){
        cout<<*it;
    }
    return 0;
}
  • Pointers require memory management, which your code doesn't have. For now just try removing all the pointers and return a plain `vector` from your function. (remove all the `*`s and `&`s and replace `->`s with `.`s; leave the `*it` as it is) – Ap31 Jun 23 '18 at 07:03
  • 1
    Possible duplicate of [Why should I use a pointer rather than the object itself?](https://stackoverflow.com/questions/22146094/why-should-i-use-a-pointer-rather-than-the-object-itself) –  Jun 23 '18 at 07:07
  • Have you tried using your debugger? What did you observe/learn while stepping through the program? – Jesper Juhl Jun 23 '18 at 10:22

1 Answers1

1

Your 1st problem is that you pass an address of a local variable to a function that expects to read the content of that address. You haven't allocated memory for that variable so it is a "temporary" variable which is saved on the stack, and not on the heap, so factor cannot access it. If you feel like unnecessarily allocating memory, you should define n as a pointer, allocate memory with "new" and pass the pointer to factor, like so:

int* n = new int;
arr = factor(n);

However, you could've easily passed factor the local variable as is, so it'll receive a copy of it. That requires you to edit your function and remove the *'s from the n's.

Your second problem is defining arr as a pointer to vector. All it does is create a variable which says : "hey, i can point you in the direction where the real vector object is at!", but there is no such object since you haven't allocated memory for it.

For this code, it is best to work with local variables, nothing here requires memory allocation, so i would suggest the following edit:

vector <int>  factor(int n){
    vector<int>  arr;
    for(int i=1;i<=n;i++){
        if(n%i==0){
            arr.push_back(i);
        }
    }
    return arr;
}
int main(){
    int n,j=0;
    vector<int> arr;
    cin>>n;
    arr = factor(n);
    for(auto it=arr.begin();it!=arr.end();++it){
        cout<<*it;
    }
    return 0;
}
CodeHoarder
  • 272
  • 1
  • 11