0

I am new to coding in C++ and am running into a very basic problem. The code below is a minimal example of the problem.

Basically, I am trying to append to a vector of pointers. In the code below, the desired result is three distinct pointers in the vector. However, whenever I add a pointer to the vector, it overwrites all entries of the vector to contain that same pointer.

The output I get is:

121
-500 -500
30 30 30

The output I want is:

121
121 -500
121 -500 30

It seems really stupid, but I am not sure how to go about fixing it! Any help would be appreciated.

#include <iostream>
#include <vector>
#include <array>
#include <iterator>
#include <list>

static std::vector<std::array<int, 2> *> heap_array;

void insert_into_heap(int num){
    static std::array<int, 2> test;
    test[0] = num;
    heap_array.push_back(&test);
}

void print_heap(){
    std::vector<std::array<int, 2> *>::const_iterator it;
    for(it = heap_array.begin(); it != heap_array.end(); ++it){
        std::cout << (*(*it))[0] << " ";
    }
    std::cout << "\n";
}

int main(int argc, char** argv){
    insert_into_heap(121);
    print_heap();
    insert_into_heap(-500);
    print_heap();
    insert_into_heap(30);
    print_heap();
    return 0;
}

As an aside, the heap-related naming is not relevant at this stage.

mchapman
  • 3
  • 1
  • 6
    do you know what a function local static does? did you add static because it was crashing when the array *was not* static? – Borgleader Feb 20 '20 at 22:10
  • 1
    You probably should get rid if the pointers. `static std::array test;` is not a solution to your problem. The address of test will be the same every time you call `heap_array.push_back(&test);` – drescherjm Feb 20 '20 at 22:11
  • Are you sure you want a `vector` of pointers? `vector` generally works a lot better if it directly contains objects and the objects are self-managing ([Rule of Three, five, or Zero observant](https://en.cppreference.com/w/cpp/language/rule_of_three)). – user4581301 Feb 20 '20 at 22:12
  • "did you add static because it was crashing when the array was not static?" I'm in this comment and I don't like it. – JohnFilleau Feb 20 '20 at 22:13
  • @Borgleader I added the local static because without it, the results are garbage. My understanding is that without the local static, the arrays wouldn't persist in memory after the function call. – mchapman Feb 20 '20 at 22:14
  • With the static your `&test` will be the same address every element in your vector will have the same pointer. – drescherjm Feb 20 '20 at 22:15
  • @drescherjm I really do need an array of pointers to objects, not actual objects. For the larger project I'm working on. Edit: thanks. I understand the source of the problem now, just not how to fix it. – mchapman Feb 20 '20 at 22:16
  • 1
    Unfortunately by making the variable persist via `static` you now only have one allocation and you're re-using it. – user4581301 Feb 20 '20 at 22:20
  • I'd look up dynamic allocation. I think it's quite likely you could side step using pointers, though if they are the right tool for your project you'd probably be better off using smart pointers. – George Feb 20 '20 at 22:22
  • As several people already said, your issue is that you create an unique instance of your `static std::array test;` variable, by declaring it static, and then you're simply reusing it again and again, by adding the same pointer to your vector. If you want to put a different pointer in your vector, you need to create a new instance, e.g by using the `new` keyword. And of course you'll need to delete the content of your vector before the application ends. – Jean-Milost Reymond Feb 20 '20 at 22:34
  • Do not recommend `new`/`delete`. The path of `new`/`delete` looks simpler, but [down that road lies madness](https://stackoverflow.com/questions/6500313/why-should-c-programmers-minimize-use-of-new). If pointers are truly necessary it sounds like a good case for `std::shared_ptr` can be made. If not, `std::unique_ptr`. – user4581301 Feb 20 '20 at 22:57

2 Answers2

3

Explanation

void insert_into_heap(int num){
    static std::array<int, 2> test;
    test[0] = num;
    heap_array.push_back(&test);
}

In this function you declare test as static, which means it creates this array once the function first called, and only update it at the following times. When you send this test's address to your heap, you actually send the exact same address every time. Assume the address of this test was 0x1234 in the first call, it will remain this way for all of the rest calls. So, when you update each time this static variable, you actually update the whole heap array elements each time (because they are all pointing to the same memory location).

How to solve?

There are several ways you can go through this issue. I would just recommend you to use smart pointers in your heap_array, and make_shared/make_unique in your insert_into_heap function, so you will create some real new memory locations on the heap.

References:

Static key word
Smart Pointers

CoralK
  • 3,341
  • 2
  • 8
  • 21
1

In the code below, the desired result is three distinct pointers in the vector.

They are three distinct pointer, but they all do point to the same array. There is only a single array in your code:

void insert_into_heap(int num){
    static std::array<int, 2> test;      // <-----------
    test[0] = num;
    heap_array.push_back(&test);
}

However, whenever I add a pointer to the vector, it overwrites all entries of the vector to contain that same pointer.

That is not really what happens. The vector heap_array is not the problem, but test[0] = num; writes to the same element on each call. And when you print, you print the same array several times.

I suppose you declared test as static because otherwise the pointers you store in heap_array point to no longer alive objects. However, it is not clear why you want to have pointers in heap_array in the first place. The arrays need to live somewhere. Using a function local static is not the solution. Store objects instead of pointers and your problem is gone.

I really do need an array of pointers to objects, not actual objects. For the larger project I'm working on

Let me reiterate: The arrays need to live somewhere. You basically have two choices: Either you store the actual objects in another container, or you store smart pointers to dynamically allocated objects.

463035818_is_not_a_number
  • 64,173
  • 8
  • 58
  • 126