[Solved]Defining Variables at start of Main-File in C++

[TheMarius97]

Hi guys

 

I have a question regarding "the beauty of code".

 

I'm doing a small text based RPG in C++ for practicing. I've only just started working on it.

 

The start of my main.cpp looks like this:

#include <iostream>#include <string>int swords, sandwiches;#include "items.h"using namespace std;

I know (from testing) that this works (the integers are used in items.h).
.

 

Is it bad practice to do it that way? Are there other ways (besides defining them in items.h)?

 

 

Thanks in Advance

Greetings, Marius

[squirrl]

Not entirely bad practice. It just depends how you are using them. It could be more reasonable to make them static inside a class though.

[JamieF]

It's probably best to declare the variables when you need them so you don't have to keep scrolling to the top and so you can see what's happening to it once it's declared.

 

It should work throughout the program, not just in the headers.

[Avratz]

If you must use globals, namespace them and define them as extern. 

 

This will prevent the possibility of multiple definitions of a variable when including headers. extern only gives a file access to the variable and tells the compiler the variable exists somewhere. The linker will figure out where later.

 

namespacing will help keep things organized, and help against accidentally using a local var instead of the global (or vise versa).

//globals.h//this just declares the existence of g_Foonamespace GlobalDefs{    extern int g_Foo;}//somefile.cpp//the actual definition of g_Fooint GlobalDefs::g_Foo;//someotherfile.cpp#include "globals.h"int foo(){    //we can use g_Foo even though the actual definition resides in another file    GlobalDefs::g_Foo = 1;}

[WanderingFool]

Just going to weigh in on this because there is something that hasn't been said yet.  Not going to give answers though since I agree with Avratz.

 

So just from what I understand in your post, you are using a variable that is defined in a cpp and then using it in an header file. (Which is why I guess you had to define it prior to calling the include...I am actually surprised this compiles, perhaps I am misunderstanding what your code actually looks like)

Never use a variable from an cpp and use it in an header file.

 

I say this because when projects get larger, you may decide to include the header in other cpp files.  This would cause unexpected consequences (and compile errors).  Avratz's solution is the best....I run by the belief that header files should very much be self-contained, so no code (just definitions)...unless it is very simple like create an empty constructor...but nothing that relies on external variables....I feel that it is best to keep that stuff in cpp files, that way the variables can't be accessed by other cpp files by mistake.

[TheMarius97]

So just from what I understand in your post, you are using a variable that is defined in a cpp and then using it in an header file. (Which is why I guess you had to define it prior to calling the include...I am actually surprised this compiles, perhaps I am misunderstanding what your code actually looks like)

Never use a variable from an cpp and use it in an header file.

So I use my header file for this: I also have other Items but just as an example

 

items.h

using namespace std;class Items {	public: virtual void take() = 0;	public: virtual void drop() = 0;	public: virtual void drink() = 0;	public: virtual void eat() = 0;};class Coffee : public Items {	public: void take(){		cout << "Coffee taken" << endl;		coffees++;	}		public: void drop(){		cout << "Coffee dropped" << endl;		coffees--;	}		public: void drink(){		cout << "Coffe drunk" << endl;		coffees--;	}		public: void eat(){		cout << "You can't eat coffee!" << endl;	}};

As you can see, I edit the integer variable 'coffees' in these methods in my main.cpp I then have this code(snippet)

 

main.cpp

int coffees, sandwiches, deer;#include "items.h"void take(Items *item){	item->take();}void drop(Items *item){	item->drop();}void drink(Items *item){	item->drink();}void eat(Items *item){	item->eat();}int displayStats(){	system("CLS");	cout << "Cups of coffee:\t\t" << coffees << endl;}int main(){	Coffee coffee;	coffees = 0;	take(&coffee);	drink(&coffee);	displayStats();}

I'm now just building a frame for easier "developing" a game with it later so it's all just testing and fooling around.

But as I see it, I should not have any code in the header file, apart from the class definitions?

I'm also pretty sure that this is neither the most efficient nor the most elegant way to solve this.

 

I'll try out Avratz's solution tonight when I'm home.

 

I've really only just started so I'm sorry if these are newbie questions.

[WanderingFool]

So it is a very bad idea to allocate variables like that.  Once the project gets to any decent size it will be unmanageable.  Again if multiple cpp files call header files that require the coffee variable then it is sometimes just luck of the draw which gets compiled first, meaning it might compile, but it might not.

 

So there is a few things I will comment on now I can see your header file, some of it is my opinion of best practices (to each their own) and others are still my opinion of best practices but you really should be following these ones.

 

Well lets get to the light-hearted practices.  In C++ you can actually declare public once and you don't have to keep doing it (example below all bundled together).  Another practice, which I even break, is keeping function implementations out of the header file.  Usually you create a separate cpp file which contains the implementations (named the same).  This also has the benefit that you can shove things like variables into the cpp file, without using them in the header file (as it is all self contained in the cpp file).  Although in this case you should probably make it a class variable (I will show both ways in the example).

One last little practice would be to define the variable in the cpp file, and not in the main function (This isn't always the case, there are times where you want to initialize at certain stages, but in this case you could do it prior to main being called).

 

*In general the header should only be including what is needed to compile and understand the class you are making, nothing more*

 

So now for practices you should be following (some aren't even opinions, some are you must follow if you want the project to grow anymore in size).

So header guards, header guards are protections put in header files to prevent them from being included more than once (and causing issues).   I will show them in the example and point them out, but you really need to be using them (Also called include guards...there is even a wiki article about it)

The next issue is the using namespace std; in your header.  Using namespace in any header file is frowned upon.  By calling using namespace you are actually making it so any following cpp files that call the header will also use that namespace.  This can cause undesired consequences later on (if someone using the header file without realizing namespace was called, or if there was a conflict in names)

 

So here is my suggestion of what your code should look like after the changes (although it isn't really fully correct as I wanted to give examples of different ways).  Excuse any errors I did this in notepad without a compiler, and I haven't been keeping up with C++ as much as I would like

 

Items.h

#ifndef _ITEMS_H_#define _ITEMS_H_ //So the above is an header guard.  It will prevent the compiler from compiling the header more than once.  This is very important to have in ALL header files//just change the name of the guard...eg. _MYHEADER_H_class Items {public: //Notice only defining it public once...everything following this 	virtual void take() = 0;	virtual void drop() = 0;	virtual void drink() = 0;	virtual void eat() = 0;//So there is a design choice which I am not mentioning because I don't really have the time to discuss it and I think it is a bit much given everything else//I am trying to put across...in short though instead of the counter being in coffee if you move it to here.  Then you don't have to keep creating new variables//to do the same thing...but this requires a bit more initial work (if you had 5+ different items it would be worth while looking into this)  I will explain this//in another post when I get more time and have the energy};class Coffee : public Items {public:	void take(); //These will be defined in the items.cpp file	void drop();	public: void drink(); //You can still define it with public: like this but I personally like it without, looks cleaner	void eat();	Coffee(): currentCoffeeCount(0) {}; //So this is the only implementation I personally usually do.  This is because it is very short	//if I needed to do work in it though I would move it into items.cpp.  As another note, notice I am initializing the currentCoffeCount variable	//here.	static int getTotalCoffees() { return currentCoffeeCount; }; //Okay so I lied, I do getters in here too   I added this to show later on	int currentCoffeeCount; //I renamed this because I needed more variables to show different approachesprivate:	static int totalCoffees; //This is a new variable, notice it is private so only the class coffee will be able to access this variable	//The above variable is quite different from currentCoffeeCount.  First only Coffee can access it, the next is the static will mean	//that there will only be one of it.  This means it tracks multiple coffee objects for their totals	//Notice that this variable has to be initialized in items.cpp it can't be initialized in the header, or even in a function it has to be	//in a cpp file};//Need to close the guard#endif

Items.cpp

//Items.cpp#include "Items.h"using namespace std; //It is safe here, because this namespace won't be able to spread as it is a cpp fileint Coffee::totalCoffees = 0; //This is where totalCoffees is initialized.  So you don't have to worry about it in any other cpp files//Sorry if any syntax is wrong, I haven't been doing C++ very much in the last year//This is the implementation of the Items definitionsvoid Coffee::take() {	cout << "Coffee taken" << endl;	currentCoffeeCount++;	totalCoffees++;}void Coffee::drop() {	cout << "Coffee dropped" << endl;	currentCoffeeCount--;	totalCoffees--;}void drink() {	cout << "Coffee drunk" << endl;	currentCoffeeCount--;	totalCoffees--;}void Coffee::eat() {	cout << "You can't eat coffee!" << endl; //You obviously never have frozen a coffee in the freezer before}

Main.cpp

//Main.cpp#include "items.h"//The items.cpp handles all the initializations, so all I need to do to use the items class is call the include :)using namespace std;void displayStats(const Coffee &coffee); //Sorry I just like having main initialized first, there is nothing wrong with that you did though//Although I did need to pass the coffee through as it contains the counts :pint main() {	Coffee coffeeDispensor1;	Coffee coffeeDispensor2; //Creating two to make 	coffeeDispensor1.take(); //This is so much easier to call than your other methods	coffeeDispensor1.take();	coffeeDispensor1.take();	coffeeDispensor1.drink();	coffeeDispensor1.drop(); //1 coffee	coffeeDispensor2.take(); //This is so much easier to call than your other methods	coffeeDispensor2.take();	coffeeDispensor2.take();	coffeeDispensor1.drop(); //2 coffees for this one, but there is a total of 3 coffees out there	system("CLS"); //Clearing screen here	displayStats(coffeeDispensor1);	displayStats(coffeeDispensor2);	//ANd printing out the global amount	cout << "Total cups of coffee:\t\t" << Coffee.getTotalCoffees() << "\n"<<endl;}void displayStats(const Coffee &coffee) {	cout << "Cups of coffee:\t\t" << coffee.currentCoffeeCount << "\n"<<endl;}

There are more design choices that I could talk about, but for now I will leave it at this. (I don't want to flood with too much information)

[TheMarius97]

Thank you so very much!

This is one of the best if not the best answer I got so far on any forum.

Thank you for taking your time.

 

I will apply these tips,(practices) on the weekend.

 

Off Topic: Is frozen coffee any good in these hot summer days?

[WanderingFool]

No it is not...I personally hate coffee, which is why licking it thinking it is Cola flavored is really bad.

 

Anyways I have a bit more time, also I was bored, so here is the second part where it is improving.  Basically the concept is you want to reuse the items as much as possible...so items is a generic counter...I also am renaming it to itemCounter.  I am will be surprised if this one compiles, but the concept is there.

 

 

Items.h

#ifndef _ITEMS_H_#define _ITEMS_H_ class ItemCounter {public:	virtual void take(); //You can override these, but they aren't abstract methods anymore	virtual void drop();	virtual void drink();	virtual void eat();	virtual string getName() = 0; //Added this, because I need a name	virtual string getPluralName() = 0; //Added this, because I need a name	virtual void printNumberDescriber(); //This will make a generic "Cups of [name]" or "[name] eatten", you can override if it doesn't fit the specific	int _itemCount; //Really I should be using an gettor for this instead of just making it public...but I am too lazy to write the extra code at the moment	//I am using _ here because I am going to start using a coding convention that I am use toprotected:	//Basically initializing everything.  The reason for _ is I don't confuse a local vs class variable.   As a note I protected the constructor so someone	//won't make a generic itemcounter	ItemCounter(bool canDrink, bool canEat): itemCount(0), _canDrink(canDrink), _canEat(canEat) {};	bool _canDrink, _canEat; //Is it bool or boolean?  I can't remember the variable type };//Moved Coffee to ItemTypes.h#endif

Items.cpp

//Items.cpp#include "Items.h"using namespace std;void ItemCounter::take() {	cout << getName() << " taken" << endl;	_itemCount++;}void ItemCounter::drop() {	cout << getName() << " dropped" << endl;	_itemCount--;}void ItemCounter::drink() {	if(!_canDrink) {		cout << "You can't drink a " << getName() << endl; //Probably should be calling to lower but I can't remember the function name		return;	}	cout << getName() << " drunk" << endl;	_itemCount--;}void ItemCounter::eat() {	if(!_canEat) {		cout << "You can't eat a " << getName() << endl;		return;	}	cout << getName() << " eatten" << endl;	_itemCount--;}void ItemCounter::numberDescriber() {	if(_canDrink && _canEat) {		cout << getPluralName() << " consumed: " <<  _itemCount << endl;	}	else if(_canDrink) {		cout << "Cups of " << getPluralName << ": " <<  _itemCount << endl;	}	cout << getPluralName << " eatten: " <<  _itemCount << endl;}

ItemTypes.h

#ifndef _ITEMTYPES_H_#define _ITEMTYPES_H_#include "items.h"//This actually means if you have 2 coffee counters you would have to manually add up the coffees, and this class no longer//has the variable holding the global coffee count...but with that said you could always create callback function to//make it so all ItemCounter children get told when a new one is make or destroyed. (Just add virtual void addCount, and call it in take for itemCounter)//But that is too much writing for me at the moment, so it is not includedclass CoffeeCounter : public ItemCounter {public:	//Notice the take, drop, drink and eat function are gone...the ItemCounter handles it now, but now there is a new function	string getName() { return "Coffee"; }	string getPluralName() { return "Coffee"; } //I am only doing this so the grammatical syntax will be correct	//Initialize the parent, and pass in whether you can drink or eat coffee	CoffeeCounter(): ItemCounter(true, false) {};};//Now you can see the benefit of this, I present to you copy and pasting of 6 lines of code that allows me to create multiple objectsclass SandwichCounter : public ItemCounter {public:	string getName() { return "Sandwich"; }	string getPluralName() { return "Sandwiches"; }	SandwichCounter(): ItemCounter(false, true) {};};class JelloCounter : public ItemCounter {public:	string getName() { return "Jello"; }	string getPluralName() { return "Jellos"; }	JelloCounter(): ItemCounter(true, true) {}; //Yay for jello being both liquidy and solidy};//So this next bit is a very dangerous thing to learn, and can cause a lot of problems if used wrongly...but when you have multiple classes that you want to//be made that are essentially clones of each other this actually is powerful...and why I fell in love with C/C++ initially.  I do not recommend doing this though//but man it can save a lot of typing...and I hope I did this right, it has been years since I did this type of macro#define DefineNewItemViaMacro( __ClassName__, __Name__, __Plural__, __Drinkable__, __Edible__ ) \class __ClassName__##Counter: public ItemCounter { \public:\string getName() { return __Name__##; } \string getPluralName() { return ##__Plural__##; } \__ClassName__##Counter: ItemCounter(__Drinkable__, __Edible__) {}; \}//Uhm if I got this ugly mess working correctly then the following is valid syntax :p//That is a big IF though :pDefineNewItemViaMacro(Sushi, "Sushi", "Sushi", false, true);DefineNewItemViaMacro(Plastic, "Plastic", "Plastic", false, false);DefineNewItemViaMacro(Grass, "Grass", "Grasses", false, true);DefineNewItemViaMacro(Water, "Water", "Water", true, false);//See 4 lines, 4 new item classes  I hope#endif

main.cpp

//Main.cpp#include "itemTypes.h"int main() {	CoffeeCounter cc;	JelloCounter jc;	cc.take();	cc.take();	jc.take();	system("CLS"); //Clearing screen here	cc.printNumberDescriber();	jc.printNumberDescriber();}