Is it ok to assign this to a static pointer in constructor as in example here

Question

I am using the approach below for convenience. ie a convenient means of accessing the same myapp instance in a larger program. The code compiles and runs correctly on my machine but want to ask if anyone sees any problems with this approach?

For example, the this ptr is assigned to the_app in the constructor? Is that ok? My concern is that object is still being constructed. but if last line of constructor then ok? Or is it because it is a pointer so doesn't matter because as long as used when fully constructed will be ptr to full object?

#include <iostream>

using namespace std;

class myapp
{
public:
   myapp() : m_data(0)
   {
     the_app = this;
   }

   void DoIt() { cout << "doing it\n"; }

   static myapp* the_app;

private:
   int m_data;
};

myapp* myapp::the_app = 0;

int main(int argc, char* argv[])
{
myapp app;

    app.DoIt();  //doing it using member function

    myapp::the_app->DoIt();  //accessing using static ptr
return 0;
}

Language note: I usually read "a = b" as "b is assigned to a" rather than "a is assigned to b". — wolfgang, Apr 04 '12 at 08:56

score 1 · Answer 1 · answered Apr 04 '12 at 08:50

This is working as expected. BUT there are a couple of issues:

When the instance is deleted, the destructor should set the class variable back to zero. You didn't write about that.
When another instance is created, the class variable changes and maybe this leads to the first instance getting lost...
It does not seem like a good idea (just my personal impression)

score 1 · Answer 2 · edited May 23 '17 at 12:07

1

If what you are after is static access to a single instance of a class, then you might want to check out the C++ Singleton design pattern

edited May 23 '17 at 12:07

Community

1
1

answered Apr 04 '12 at 09:01

grim

143
1
7

Yes I think this is probably best way to go – Angus Comber Apr 04 '12 at 11:27

score 0 · Answer 3 · answered Apr 04 '12 at 08:47

0

It's not ok:

{
   myapp app;
}
myapp::the_app->DoIt();

This would be illegal, as myapp::the_app is a dangling pointer at this stage.

answered Apr 04 '12 at 08:47

Luchian Grigore

253,575
64
457
625

littleadv · Answer 4 · 2012-04-04T08:54:21.427

0

If you mean to initialize it, then no, you must explicitly define your static member variables and initialize them there:

If the data member is to be explicitly initialized, an initializer must be provided with the definition

There's only one per class, and is shared between all the objects. Static data members are accessed through static function members.

If you didn't mean to initialize, then you need to take into the account that each new instance of your class overwrites the value, and you always have the last pointer created. When your class is destructed - that pointer may become invalid.

edited Apr 04 '12 at 08:54

answered Apr 04 '12 at 08:47

littleadv

20,100
2
36
50

He does do that - `myapp* myapp::the_app = 0;` – Luchian Grigore Apr 04 '12 at 08:50
"No, you must explicitly define your static member variables and initialize them" He is doing that. – Luchian Grigore Apr 04 '12 at 08:52
@LuchianGrigore better now? I was sure that people would read past the first sentence, but this assumption consistently doesn't hold with you:-/ – littleadv Apr 04 '12 at 08:54
When I read the answer, it only had the first part. Don't take it personally. – Luchian Grigore Apr 04 '12 at 08:56

score 0 · Answer 5 · answered Apr 04 '12 at 08:47

Problems I can see:

If you subclass myapp, the subclass won't be fully constructed when the pointer is assigned.
The static pointer isn't cleared in the myapp destructor because you haven't written one. The pointer might live on after the object has been destroyed, if your real code doesn't have it on the stack of main().
Multi-threading: If you are using multi-threading then you have a problem, since the assignment may be re-ordered before the rest of the myapp initialisation. Also assigning the pointer might not be atomic depending on the platform.

score 0 · Answer 6 · answered Apr 04 '12 at 09:11

As others have pointed out, problems are:

a new instance could be created at some point, unexpectedly replacing your my_app instance
a dangling pointer remains when the instance is destroyed
when you derive from my_app, the pointer is assigned before the object is fully constructed.

You could alleviate the first two of these problems by adding

assert(the_app == NULL);

to your constructor and

the_app = NULL;

to your destructor.

Alternative solutions that come to mind:

std::scoped_ptr<my_app> the_global_app;

... if you use a single global object, you might as well admit that you are using a global variable. In your main function:

int main(int argc, char* argv[])
{
    the_global_app.reset( new myapp() ); // or a derived class

    the_global_app->DoIt();  //accessing using static ptr
    return 0;
}

If you don't intend to derive from myapp, you might even be tempted to use a plain global variable:

myapp the_global_app;

int main(int argc, char* argv[])
{
    the_global_app.DoIt();  //accessing using static ptr
    return 0;
}

... but then you need to be aware that other global objects defined in different source files might not have been initialized (std::cout is guaranteed to be available, though).

Is it ok to assign this to a static pointer in constructor as in example here

6 Answers6