2

I made a class which has a static member in it. Every time an object is created I want to add the pointer to this object in the static member.

Header file:

#include <vector>

class A
{
public: 
    A(const int pin);
    ~A();
    static std::vector<A*> channels;

private:
    int pin_number;
}

cpp file:

 #include "A.h"

 std::vector<A*> A::channels;

 A::A(const int pin) : pin_number(pin) { A::channels.push_back(this); };
 A::~A() { 
     std::vector<A*>::iterator ch = std::find(A::channels.begin(), A::channels.end(), this);
     if (ch != A::channels.end()) {
        A::channels.erase(ch);

 }

In the main.cpp I want to declare the object. However, when I declare and initialize as a global variable it doesn't seem to work:

A a1{ 1 };
A a2{ 2 };

int main() {
    std::cout << "Vector size: " << A::channels.size() << std::endl;
}

The above mentioned code doesn't seem to work. In the constructor of the object I see the vector being filled. The output is 0 in the above case.

For the 2 code samples below, however, does seem to work.

Sample 1:

A *a1;
A *a2;

int main() {
    a1 = new A{ 1 };
    a2 = new A{ 2 };

    std::cout << "Vector size: " << A::channels.size() << std::endl;
} 

Sample 2:

int main() {
    A a1{ 1 };
    A a2{ 2 };

    std::cout << "Vector size: " << A::channels.size() << std::endl;
}

In the above 2 cases it prints 2, which is what I expected.

Can anyone help me explain what i'm doing wrong, or what I'm missing. I'm guessing it has to do something with the scope of the objects but I can't seem to reason why the first example doesn't work.

Edit: I didn't add the destructor for the class since I didn't think it is relevant for this question.

Bob
  • 149
  • 1
  • 1
  • 7
  • Remove the stars from your Sample 1. –  Oct 25 '18 at 20:06
  • 3
    How this compiles? `A *a1{ 1 };` – Slava Oct 25 '18 at 20:07
  • Unrelated, but I don't see you dynamically allocating anything, so I'm not sure why you feel you'd need a destructor. – scohe001 Oct 25 '18 at 20:08
  • @J.Doe : Why would i need to do that? – Bob Oct 25 '18 at 20:08
  • 1
    please provide a [mcve]. you need to define the vector somewhere – 463035818_is_not_an_ai Oct 25 '18 at 20:09
  • @Slava : Sorry, my mistake, edited the post. Removed the stars. – Bob Oct 25 '18 at 20:09
  • 1
    @Bob "Why would i need to do that?" because C++ is not Java and pointer is a different thing than an object in C++. – Slava Oct 25 '18 at 20:10
  • Can't find a good dupe right now but what you are seeing is called the static initialization order fiasco. – NathanOliver Oct 25 '18 at 20:10
  • @Bob Sorry, meant your Sample 0, the one before Sample 1. –  Oct 25 '18 at 20:10
  • 1
    Where is this static vector **defined**? This is a key question. Wild guess - likely **after** static objects. – SergeyA Oct 25 '18 at 20:12
  • @scohe001 : Sample 1 does this right? – Bob Oct 25 '18 at 20:12
  • 1
    fyi, `channels` in your example is not initialized. Because it's a non-const static in your class you have just a declaration, not a definition. So your example shouldn't even compile – bolov Oct 25 '18 at 20:13
  • @Bob sure, but `main` is allocating the memory, so `main` is in charge of cleaning it up, not your `A` class. `A` doesn't allocate anything dynamically, so what would it cleanup in the destructor? – scohe001 Oct 25 '18 at 20:14
  • @bolov : i just edited the post. I have described the header and cpp file in there. – Bob Oct 25 '18 at 20:23
  • @scohe001 : In this case I have no idea how to do this, since this is a teensy 3.6 which has a main function but the program never stops unless you turn it off? – Bob Oct 25 '18 at 20:40
  • 1
    @Bob what I mean is that it's *your* job **in main** to cleanup what you've allocated **in main**, does that make sense? Once you're done with the objects you `new`'ed, `delete` them! – scohe001 Oct 25 '18 at 20:43
  • @scohe001 : Thanks, i know that much but this is a microcontroller so i don't know how to clean it up since it is just powered off, or am i missing something? – Bob Oct 25 '18 at 20:45
  • 1
    @Bob don't worry about trying to cleanup during a power-off. On power-off, all your memory is wiped after all, so there's not much better cleanup than that :P I'm talking more about mid-program if you're done with an object or are planning to point the pointer elsewhere, you'll need to deallocate to avoid memory leaks (or just don't use pointers in the first place and then you won't have to deal with this hassle!) – scohe001 Oct 25 '18 at 20:49
  • @scohe001 : Thanks for the help! Then I'm ok with it. I know about the pointers needing cleanup I purposly left it out since it wasn't really relevant. Thank you very much! – Bob Oct 25 '18 at 20:54

2 Answers2

8

With global object you have classical problem - order of initialization of global objects from different compilation units is undefined, so in this case it seems your vector initialized after those 2 instances are created. Solution is to have static local vector:

class A
{
public: 
    A(const int pin) { A::channels().push_back(this); };

    static std::vector<A*> &channels() 
    {
        static std::vector<A*> theInstance;
        return theInstance;
    }
}

so your issue with initialization order would be resolved.

Note: if it is unclear why this solution works you can look into this answer - your problem is related to problem of implementing singleton pattern and it is deeply addressed there.

Slava
  • 43,454
  • 1
  • 47
  • 90
  • @NathanOliver you fixed it couple seconds before I tried to submit the same change :) Thanks. – Slava Oct 25 '18 at 20:18
  • You're welcome. Not sure why this got down voted though :( – NathanOliver Oct 25 '18 at 20:18
  • @Slava : Would you mind explaining a bit more about this? What does this static local vector do and how does it work? – Bob Oct 25 '18 at 20:27
  • 1
    @Bob unlike global objects, local static objects initialize when execution flow goes over it's definition the first time. This is common problem for implementing singleton, details can be found [here](https://stackoverflow.com/questions/1008019/c-singleton-design-pattern) – Slava Oct 25 '18 at 20:56
1
A a1{ 1 };
A a2{ 2 };

int main() {
    std::cout << "Vector size: " << A::channels.size() << std::endl;
}

The A a1 { 1 }; and A a2 { 2 }; has the objects created globally. Where as the pointer version has the pointers created globally, but the objects created on the heap not in global space. This is the same for your other example. The channels member in class A is static to A.cpp, but the objects are being created in Main.cpp globally. This means if you want the count to update properly, you'll have to put std::vector<A*> A::channels; into the main.cpp and not the A.cpp

  • Sorry but i don't understand your first sentence, could you explain this a bit more? – Bob Oct 25 '18 at 20:37
  • @Bob The first 2 lines of code in that code block are global, they aren't declared in any functions but outside of functions or classes. This puts it in a different space in memory known as a [data segment](https://en.wikipedia.org/wiki/Data_segment). – JComputerScience Oct 25 '18 at 20:40