views:

631

answers:

8

I'm having some trouble compiling/linking a set of classes, several of them dealing with a common global variable.

Basically, I declare and define a extern variable foo in class A and access/update it in classes B and C.

The relevant code looks like this:

A.h

extern string foo; // declare it <=== compiler error "storage class specified for foo"

B.cpp

include A.h  
string foo; // define it  

main () {   
...  
foo = "abc";      
}

C.cpp

include A.h  
cout << foo; // print it

My current error is "storage class specified for foo". But, I'm wondering if this is the correct approach. Should I be using a static variable? Any help much appreciated, as I've been on this for at least an hour by now.

+4  A: 

Do it on file level. Outside main()

string foo;

int main() {

}

Otherwise it is not global at all, but rather "auto"

EFraim
Thanks, but no luck. It still give me the "storage class specified for foo" message on the external declaration.
Jack BeNimble
Don't include "A.h" in B.cpp. Only include it in C.cpp.Actually you shouldn't put an extern declaration into a header file.Better just put the extern string foo; at the top of cpp files that are using it.And still better: don't use extern. ;) See my answer for details.
haffax
@haffax: I don't completely agree. Having a bunch of constant strings (or any other non-trivial objects) to be shared would be an obvious thing to have. In that case you would have a header listing them all with 'extern' on each and a source file with the actual definitions. Using singletons would be massive overkill in that case. Even one singleton containing the whole lot would still make accessing them pointlessly verbose. For non-constant variables though I agree it should probably be handled in a neater way as you suggest.
Troubadour
@Troubador, the original question says explicitly that foo is accessed/updated by B and C. And even if it is a constant string, I'd either let the compiler handle the const-aspect (It will unify constant strings) or I'd otherwise make the dependency of B and C to string explicit. Singleton was just an example. There are many ways to resolve this. String can be given as an argument to the ctor, or init method. Or the string could be an attribute of some common dependency of B and C. Both would be preferable to using an extern-reference to a global looking from an OOP perspective at the problem.
haffax
@haffax: Your comment about not putting extern in a header was clearly offered as a generic piece of advice. As such that it is how I responded to it in my comment.
Troubadour
A: 

When you are defining foo, you are defining a local variable in the function main. When you link, you will get a missing symbol because your extern foo; is never created. You need to have foo defined outside a function.

b.cpp

string foo;
int main () {
   foo = "abc";
}

Better yet, you should try a singleton.

David Nehme
+1  A: 

Other than moving the definition outside of main, make sure you are including the string header before your extern declaration:

#include <string>
Jim Buck
Jim - I am including the extern declaration in B.cpp. B.cpp includes A.h which has the extern declaration.
Jack BeNimble
Ack, markdown left out some text. I meant to include the STL string header before your extern.
Jim Buck
Sadly I think he ditched his question. Unfortunate, because I think we are right.
GMan
Actually, looking over his own answer, I think he tried to "extern" something that lived inside a class (like a static member), though it wasn't clear from his code samples that that was what he was doing. Troubadour seemed to have glommed onto this.
Jim Buck
+2  A: 

While EFraim's answer is correct your other question is, whether this is the correct approach. Answer: No, in most cases.

Having a single global value that is manipulated by multiple classes is just begging for problems: Using extern is pretty subtle. There are no clear hints for any programmer looking at your code where the variable is defined. He/she has to do a full source scan to find it. Also it is difficult to determine when what class changes the value in the control flow of your application, a global variable ties different classes together with an invisible band. A good design makes collaborations between classes explicit.

Better: Make your global a singleton. This way you at least know where the value is defined and you can control changes to the value via access methods. Even better: Work out why the classes have to have access to the same value, work out which class depends on what aspect exactly and revise your design accordingly. Often enough using extern is just a quick fix for a deeper design problem.

haffax
I actually did go with the singleton, although that was more of a hassle than I thought it would be. I need to either code more C++ or not at all! Thanks1
Jack BeNimble
+4  A: 

Since your error is on the extern, I'm guessing it doesn't know what the type is.

Have you included string?

#include <string>

And if so, you need to put std:: before it:

#include <string>
extern std::string foo;

Side note, make sure you don't use any using directives (using namespace std or using std::string) in your header file, because then you force everyone who sues your header file to do the same, which is bad practice.

Edit

...but that's how I have it coded.

Are you sure? I just tried this and it works completely fine in both VC++ and g++:

A.h

#include <string>

extern std::string foo;

B.cpp

#include "A.h"

std::string foo;

int main (void)
{   
    foo = "abc";      
}

C.cpp

#include "A.h"
#include <iostream>

int some_function(void)
{   
    std::cout << foo << std::endl;
}

Try that out and see if it works.

GMan
Thanks GMan, but that's how I have it coded.
Jack BeNimble
It can't be, because the code is well-formed and compiles.
GMan
GMan, I actually went ahead with the singleton code and just now finished it. I would've tried it, but I figured it would be better just to learn the singleton than extern. Thanks for your help :)
Jack BeNimble
A: 

Well, tt looks like C++ doesn't let you use "extern" on anything but objects or functions:

from http://publib.boulder.ibm.com/infocenter/comphelp/v8v101/index.jsp?topic=/com.ibm.xlcpp8a.doc/language/ref/extsc.htm

"C++ restricts the use of the extern storage class specifier to the names of objects or functions. Using the extern specifier with type declarations is illegal. An extern declaration cannot appear in class scope."

Back to the drawing board...

Jack BeNimble
+1 Riddle solved. I missed the "in class A" part.
haffax
-1 Riddle not solved.
GMan
I suspect the important part of the quote is actually "An extern declaration cannot appear in class scope." and the code snippet in the question is misleading.
Troubadour
+1  A: 

GMan's answer is correct: without including string, you are attempting to instantiate an undefined type, thus the error.

The stuff quoted from the standard is not apropos: Jack is not attempting to define a type as static, he's trying to instantiate an object.

Yes you can only do stuff like

class foo {
  // ...
  static int hoo; // class variable same in all instances of foo
  // ...
};

...but that's not what jack is doing.

Why does all my code get mangled in this BBS? Last week put up an answer about C pointers and I wound up with double asterix's when I wanted single ones!

You need to put 4 spaces before your code. Also, if my answer is correct you should up-vote it :) :P
GMan
Hmm, I'm a noob on this system ;-) How do I up-vote something?
Never mind, I figured it out.
+1  A: 

In A.h have you actually got the extern declaration within the actual class A? Your question currently says this in words but then your code snippet suggests it is at file level. The only way I can get the error you are talking about i.e. "storage class specified for foo" is for the following in A.h:

class A
{
public:
    extern std::string foo;
};

Perhaps this is your problem?

Edit: Looking at your own answer I think this is what you've done. You want to replace extern with static and then define that static in A.cpp with a line like

std::string A::foo;

You can then subsequently access it in other places as A::foo eg.

std::cout << A::foo;
Troubadour
I'm not sure now, as I have gone on to code a Singleton (another story...) based on the dire warnings in this thread. Thanks:)
Jack BeNimble
Actually, I am sure. That was how I had it coded. I wish I hadn't bailed so soon, as this would've surely saved me my evening...
Jack BeNimble