Is this a valid C++ include pattern?

Question

I have C++ that follows this pattern in an include file:

#ifdef MYHEADER
extern ClassA globalA;
#endif
#ifndef MYHEADER
#define MYHEADER
class ClassA {
    // code for ClassA
    };
static ClassA globalA;
#endif

The desire is to have only one instance of ClassA (globalA) where it is defined only in the header file. This is old code that I'm trying to clean up without making mass changes.

The problem I'm seeing is that in the debugger there are (at least) two different instances of globalA (two different addresses). I searched for other declarations and even commented out the static declaration to make sure I get a link error (I did). This code is threaded.

Is this a valid pattern? What might I misunderstood? Is there a better way to do this (without requiring changes to all references of globalA) ?

static ClassA globalA; does not belong in a header. It belongs in a cpp file. Don't make instances of things in headers. — Christopher Pisz, Sep 13 '18 at 22:42
@ChristopherPisz there are occasional cases for `static` variables in headers, for example when you really do want each including translation unit to have a particular variable without defining it in each and every translation unit. Admittedly rare, but has use cases.. — user4581301, Sep 13 '18 at 22:45
@user4581301 Not on any team I will ever work with there aren't. — Christopher Pisz, Sep 13 '18 at 22:46
That's your personal choice. To me even `goto` has valid uses. — user4581301, Sep 13 '18 at 22:48
@user4581301 Feel free to start a chat and tell me all about your bad programming habits. — Christopher Pisz, Sep 13 '18 at 23:06

score 1 · Accepted Answer · answered Sep 13 '18 at 22:44

You pattern does not do what you think it does.

Translation units are compiled independently of each other. Every translation unit that wants to use globalA will need to know what ClassA is defined as. So, each time a given translation unit is compiled, MYHEADER will not be defined until your header file defines it, and as such every translation unit will end up seeing the static declaration, so every translation unit will get its own local copy of globalA, which is not what you want.

To do what you are attempting, you need to

get rid of the #ifdef block altogether.
replace the static declaration with the extern declaration.
move the globalA variable instance to one of your cpp files.

MyHeader.h

#ifndef MYHEADER
#define MYHEADER
class ClassA {
    // code for ClassA
    };
extern ClassA globalA;
#endif

MyHeader.cpp

#include "MyHeader.h"
ClassA globalA;

Is this a valid C++ include pattern?

1 Answers1