Global Variables

[brandonator]

I have been constantly told that global variables are a bad idea, because it can cause memory leaks, what are the other types of variables named, and how would i use them?

Examples would be nice, thank you.

[IceThatJaw]

Global variables can make debugging large programs difficult so it is best to avoid them.

In your design phase, if you feel the need to use several global variables then consider using classes to encapsulate them or pass around around structures to your functions.

[Aceix]

_@IceThatJaw

Yeah I've had that same caution. But I want to know whether just an int global variable, or any other type(not dynamically allocated) can cause memory leaks? If yes, is there any way I can deallocate mem(memory) for an int?


Thanks,
Aceix.

[brandonator]

i have a game created so far, and its almost 520 lines so far, i have created it with mostly all global variables and i am getting to the phase where i will have to somehow get rid of them now to prevent problems in the future, this is why i wanted to learn this.

[Chervil]

I have been constantly told that global variables are a bad idea, because it can cause memory leaks

I don't think memory leaks are the primary reason, if at all.

The main point is you are exposing the data so that any function, or any block of code, anywhere in the program, can read and/or change the variables.

It makes reading and understanding the code more difficult, as it's not immediately obvious where each variable is used.An alternative is to pass the variable as a parameter in a function call. That makes it clear where it's used, and the different type of parameter usage makes it clear both to the compiler and the reader where a variable may be changed.

The general rule is to limit the scope of each variable as narrowly as possible, to just the area where it is required.

A really trivial example:

int x;
for (x=0; x< 10; x++)
{
    ....
}

for (int y=0; y<10; y++)
{
    ...
}

In the first example, all the code following the declaration of x has access to it, whether it needs it or not.

On the other hand, y is created at the start of the for loop and is destroyed when the loop finishes. No other code has access to y, as it no longer exists.

[Aceix]

To add to Chervil, when a global variable is overshadowed in another function, it makes analysing the code confusing.

Aceix.

[brandonator]

i did this to all my global variables to try and get rid of them, but now its undeclared to all of my other functions, did i do this right and how do i solve this problem

class VariableStorage
{
     public:
VariableStorage()
{
string SetType;       
string PlayerAge;
string PlayerName;
string PlayerGender;
string CharacterDetails;
string OpponentCharacterDetails;
string Continue;
string PlayAgain;
string Type;
int Attack;
int Defense;
int Health;
string Advantage;
string Disadvantage;
string MonsterName;
int Damage;
string OType;
int OAttack;
int ODefense;
int OHealth;
string OAdvantage;
string ODisadvantage;
string OMonsterName;
int ODamage;
long Earnings = 10;
long Money = 0;
long AmountExp = 10;
long TotalExp = 0;
long NewLevel = 0;
long LevelUp = 10;
long SkillPoints = 3;
long TotalSkillPoints = 0;
int TypeOfCharacter;
}
~VariableStorage(){}
private:
};

[AbstractionAnon]

Putting everything as public in a single class really doesn't solve anything.

To solve you undeclared variable problem, you need to instantiate you class.

VariableStorage vars;

Then every reference needs to be qualified as follows:

vars.Earnings = 10;

BTW, you can't initialize values in a class declaration.
As you've coded this, you're going to need to make vars global, so as I said, your class doesn't solve anything.

What you want to do is group data elements together that logically belong together.
For example, you have a number of elements related to player.
So:

class Player
{
private:
  string age;
  string name;
  string gender;
  ... etc
};

Note that I've declared these private so they can not be accessed by code outside of the player class. You could set age, name, gender in the constructor for Player or you could have a prompt_for_player functiion that prompts for these values.

You probably want another class for Monster. Note that you've repeated some of your variables with 'O' in front of them (OType, OAttack, etc). I presume these are attributes of Monster.

Since both Player and Monster share attributes, you might want to make a base class that contains these elements. That way both Player and Monster would inherit elements and functions common to the base class.

[brandonator]

i know you may not want to read all of this, but you can you help me in doing this or at least part so i can get the hang of it? btw 'O' stands for opponent

The following is the whole code including class VariableStorage that caused the problem, without it it works.

PART 1

#include <iostream>
#include <ctime>
#include <string>
#include <sstream>
#include <cstdlib>
using namespace std;
void pause(int dur);//pauses the program for a set number of seconds
class VariableStorage
{
     public:
VariableStorage()
{
string SetType;       
string PlayerAge;
string PlayerName;
string PlayerGender;
string CharacterDetails;
string OpponentCharacterDetails;
string Continue;
string PlayAgain;
string Type;
int Attack;
int Defense;
int Health;
string Advantage;
string Disadvantage;
string MonsterName;
int Damage;
string OType;
int OAttack;
int ODefense;
int OHealth;
string OAdvantage;
string ODisadvantage;
string OMonsterName;
int ODamage;
long Earnings = 10;
long Money = 0;
long AmountExp = 10;
long TotalExp = 0;
long NewLevel = 0;
long LevelUp = 10;
long SkillPoints = 3;
long TotalSkillPoints = 0;
int TypeOfCharacter;
}
~VariableStorage(){}
private:
};
//------------------------------------------------------------------------------
class Monster
{
      public:
      // constructor
      Monster(){}
      ~Monster(){}
      //Memeber Methods      
      void DisplayStats()
      {
           cout << "\n\n\t--------Monster Stats--------";
           cout << "\n\tYou sent out a " << Type << " type.\n";
           cout << "\n\tAttack:" << Attack;
           cout << "\n\tDefense:" << Defense;
           cout << "\n\tHealth:" << Health;
           cout << "\n\tAdvantage:" << Advantage;
           cout << "\n\tDisadvantage:" << Disadvantage;
           cout << "\n\t-------------------------------\n\t";
      } 
      //Accessor Methods
      string GetType() { return Type; }
      void SetType(string x) { Type = x; }
      
      int GetAttack() { return Attack; }
      void SetAttack(int x) { Attack = x; }
      
      int GetDefense() { return Defense; }
      void SetDefense(int x) { Defense = x; }
      
      int GetHealth() { return Health; }
      void SetHealth(int x) { Health = x; }
      
      string GetAdvantage() { return Advantage; }
      void SetAdvantage(string x) { Advantage = x; }
      
      string GetDisadvantage() { return Disadvantage; }
      void SetDisadvantage(string x) { Disadvantage = x; }
      
      string GetMonsterName() { return MonsterName; }
      void SetMonsterName(string x) { MonsterName = x; }
      
      private:
      string Type;
      int Attack;
      int Defense;
      int Health;
      string Advantage;
      string Disadvantage;
      string MonsterName;
};

//------------------------------------------------------------------------------
void YourFireType()
{
Monster * YourFireType = new Monster;
YourFireType->SetType("fire");
YourFireType->SetAttack(8);
YourFireType->SetDefense(1);
YourFireType->SetHealth(40);
YourFireType->SetAdvantage("Nature");
YourFireType->SetDisadvantage("Water");
YourFireType->SetMonsterName("YourFireType");
YourFireType->DisplayStats();

Type = YourFireType->GetType();
Attack = YourFireType->GetAttack();
Defense = YourFireType->GetDefense();
Health = YourFireType->GetHealth();
Advantage = YourFireType->GetAdvantage();
Disadvantage = YourFireType->GetDisadvantage();
MonsterName = YourFireType->GetMonsterName();


system("PAUSE");
}
//------------------------------------------------------------------------------
void YourWaterType()
{
Monster * YourWaterType = new Monster;
YourWaterType->SetType("water");
YourWaterType->SetAttack(8);
YourWaterType->SetDefense(1);
YourWaterType->SetHealth(40);
YourWaterType->SetAdvantage("Fire");
YourWaterType->SetDisadvantage("Nature");
YourWaterType->SetMonsterName("YourWaterType");
YourWaterType->DisplayStats();

Type = YourWaterType->GetType();
Attack = YourWaterType->GetAttack();
Defense = YourWaterType->GetDefense();
Health = YourWaterType->GetHealth();
Advantage = YourWaterType->GetAdvantage();
Disadvantage = YourWaterType->GetDisadvantage();
MonsterName = YourWaterType->GetMonsterName();
system("PAUSE");
}
//------------------------------------------------------------------------------
void YourNatureType()
{
Monster * YourNatureType = new Monster;
YourNatureType->SetType("nature");
YourNatureType->SetAttack(8);
YourNatureType->SetDefense(1);
YourNatureType->SetHealth(40);
YourNatureType->SetAdvantage("Water");
YourNatureType->SetDisadvantage("Fire");
YourNatureType->SetMonsterName("YourNatureType");
YourNatureType->DisplayStats();

Type = YourNatureType->GetType();
Attack = YourNatureType->GetAttack();
Defense = YourNatureType->GetDefense();
Health = YourNatureType->GetHealth();
Advantage = YourNatureType->GetAdvantage();
Disadvantage = YourNatureType->GetDisadvantage();
MonsterName = YourNatureType->GetMonsterName();
system("PAUSE");
}
//------------------------------------------------------------------------------
void OpponentFireType()
{
Monster * OpponentFireType = new Monster;
OpponentFireType->SetType("fire");
OpponentFireType->SetAttack(8);
OpponentFireType->SetDefense(1);
OpponentFireType->SetHealth(40);
OpponentFireType->SetAdvantage("Nature");
OpponentFireType->SetDisadvantage("Water");
OpponentFireType->SetMonsterName("OpponentFireType");
OpponentFireType->DisplayStats();

OType = OpponentFireType->GetType();
OAttack = OpponentFireType->GetAttack();
ODefense = OpponentFireType->GetDefense();
OHealth = OpponentFireType->GetHealth();
OAdvantage = OpponentFireType->GetAdvantage();
ODisadvantage = OpponentFireType->GetDisadvantage();
OMonsterName = OpponentFireType->GetMonsterName();
system("PAUSE");
}
//------------------------------------------------------------------------------
void OpponentWaterType()
{
Monster * OpponentWaterType = new Monster;
OpponentWaterType->SetType("water");
OpponentWaterType->SetAttack(8);
OpponentWaterType->SetDefense(1);
OpponentWaterType->SetHealth(40);
OpponentWaterType->SetAdvantage("Fire");
OpponentWaterType->SetDisadvantage("Nature");
OpponentWaterType->SetMonsterName("OpponentWaterType");
OpponentWaterType->DisplayStats();

OType = OpponentWaterType->GetType();
OAttack = OpponentWaterType->GetAttack();
ODefense = OpponentWaterType->GetDefense();
OHealth = OpponentWaterType->GetHealth();
OAdvantage = OpponentWaterType->GetAdvantage();
ODisadvantage = OpponentWaterType->GetDisadvantage();
OMonsterName = OpponentWaterType->GetMonsterName();
system("PAUSE");
}
//------------------------------------------------------------------------------
void OpponentNatureType()
{
Monster * OpponentNatureType = new Monster;
OpponentNatureType->SetType("nature");
OpponentNatureType->SetAttack(8);
OpponentNatureType->SetDefense(1);
OpponentNatureType->SetHealth(40);
OpponentNatureType->SetAdvantage("Water");
OpponentNatureType->SetDisadvantage("Fire");
OpponentNatureType->SetMonsterName("OpponentNatureType");
OpponentNatureType->DisplayStats();

OType = OpponentNatureType->GetType();
OAttack = OpponentNatureType->GetAttack();
ODefense = OpponentNatureType->GetDefense();
OHealth = OpponentNatureType->GetHealth();
OAdvantage = OpponentNatureType->GetAdvantage();
ODisadvantage = OpponentNatureType->GetDisadvantage();
OMonsterName = OpponentNatureType->GetMonsterName();
system("PAUSE");
}

[brandonator]

Part 2

//------------------------------------------------------------------------------
void Name()
{
cout << "Welcome to the game!\n\nSince you are starting on your journey,";
cout << "let me know a little bit about you.\n";
cout << "What is your name? ";
getline(cin,PlayerName);
cout << "\nNice to meet you " << PlayerName << ".\n";
pause(5);
system("CLS");
}
//------------------------------------------------------------------------------
void Age()
{
          cout << "How old are you? ";
          getline(cin,PlayerAge);
          cout << "\nWow your, " << PlayerAge << "!! My guess was way off.\n";
          pause(5);
          system("CLS");
}
//------------------------------------------------------------------------------
void Gender()
{
          cout << "Are you Male, Female, or Alien? ";
          getline(cin,PlayerGender);
          pause(5);
          system("CLS");
}
//------------------------------------------------------------------------------
void PlayerCard()
{
cout << "Here " << PlayerName << " This is your I'D card I have created for you.\n";
cout << "-----------------------------\n";
cout << "| ID CARD |                 |\n";
cout << "-----------------------------\n";
cout << "| NAME:   " << PlayerName << "\n";
cout << "| AGE:    " << PlayerAge << "\n";
cout << "| GENDER: " << PlayerGender << "\n";
cout << "=============================" << "\n\n\n";
pause(8);
}
//------------------------------------------------------------------------------
void Program()
{
     cout << "Please note this program will only work if you press ENTER after each responce.\n";
     pause (5);    
     system("CLS"); 
}
//------------------------------------------------------------------------------
void YourType()
{
     cout << "What character do you want?\n\n";
     cout << "There is a fire type who specializes with fire attacks. (fire)\n";
     cout << "There is a water type who specializes with water attacks. (water)\n";
     cout << "There is a nature type who specializes with nature attacks. (nature)\n\n";
     cout << "Please enter the type of the monster you would like:";
     cin >> CharacterDetails;     
     system("CLS");
     if(CharacterDetails == "fire")
     {
     YourFireType();
     }
     else if(CharacterDetails == "water")
     {
     YourWaterType();
     }
     else{ 
     YourNatureType();
     }
     pause(10);

     system("CLS");
}
//------------------------------------------------------------------------------
void OponentType()
{ 
/* rand example: guess the number */
#include <stdio.h>

  /* initialize random seed: */
  srand ( time(NULL) );

  /* generate secret number: */
  TypeOfCharacter = rand() % 3 + 1;

    if (TypeOfCharacter == 1)
    {
    OpponentFireType();
    }
    else if (TypeOfCharacter == 2)
    {
    OpponentWaterType();
    }
    else
    {
    OpponentNatureType();
    }
}
//------------------------------------------------------------------------------
int TypeAdvantages()
{
//Fire vs Fire
     if(Type == "fire" && OType == "Fire") 
     { 
     Attack = Attack + 0;
     OAttack = OAttack + 0;
     } 
//Fire vs Water
     if(Type == "fire" && OType == "Water") 
     { 
     Attack = Attack / 2;
     OAttack = OAttack * 2;
     }
//Fire vs Nature
     if(Type == "fire" && OType == "Nature")
     { 
     Attack = Attack * 2;
     OAttack = OAttack / 2;
     }
//Water vs Fire
     if(Type == "water" && OType == "Fire") 
     { 
     Attack = Attack * 2;
     OAttack = OAttack / 2; 
     } 
//Water vs Water
     if(Type == "water" && OType == "Water") 
     { 
     Attack = Attack + 0;
     OAttack = OAttack + 0;
     }
//Water vs Nature
     if(Type == "water" && OType == "Nature")
     { 
     Attack = Attack / 2;
     OAttack = OAttack * 2;
     }
//Nature vs Fire
     if(Type == "nature" && OType == "Fire") 
     { 
     Attack = Attack / 2;
     OAttack = OAttack * 2; 
     } 
//Nature vs Water
     if(Type == "nature" && OType == "Water") 
     { 
     Attack = Attack * 2;
     OAttack = OAttack /2;
     }
//Nature vs Nature
     if (Type == "nature" && OType == "Nature")
     { 
     Attack = Attack + 0;
     OAttack = OAttack + 0;
     }
     return Attack;
}
//------------------------------------------------------------------------------
void Rules()
{ 
     cout << "Each of your characters battle.\n";
     cout << "You each alternate by taking turns. \n";
     cout << "Since you are a beginner you will go first.\n";
     pause(5);
     system("CLS");
}
//------------------------------------------------------------------------------
void Battlephase()
{ 
     cout <<"Let the battle begin!\n";
     pause(5); 
     YouAttack:
     system("CLS");
     { 
          if(Health > 0)
          {
                    {
                                Damage = Attack - ODefense;
                                if(Damage <=1)
                                {
                                Damage = 1;
                                }
                                cout << "You attacked and dealt " << Damage <<" Damage\n";
                                OHealth = OHealth - Damage; 
                                cout << "Your opponents health is " << OHealth << "\n\n";
                    }
          if(OHealth <= 0) 
                    { 
                                cout <<"You Win!"<< "\n\n";
                    }
                    pause(5);
          if (OHealth > 0)goto OpponentAttack;
          }
     }
     OpponentAttack:
     system("CLS");                    
     {
          if(OHealth > 0)
          {
                     {          
                                ODamage = OAttack - Defense;
                                if(ODamage <=1)
                                {
                                ODamage = 1;
                                }
                                cout << "Your opponent attacked and dealt " << ODamage <<" Damage\n"; 
                                Health = Health - ODamage; 
                                cout << "Your health is " << Health << "\n\n";
                     }     
          if(Health <= 0) 
                     {  
                     cout <<"You Lose :(\n\n";
                     }
                     pause(5); 
          if (Health > 0)goto YouAttack;
          }
     }
     
     system("CLS");
}
//------------------------------------------------------------------------------
void Earned()
{
     Money += Earnings;
     cout << "You earned $" << Earnings << " and " << AmountExp << " exp.\n";
     TotalExp += AmountExp;
     pause(3);
     cout << "You now have $" << Money << " and " << TotalExp << " exp.\n";
     pause(5);
     system("CLS");
}
//------------------------------------------------------------------------------
void Level()
{
            if(TotalExp >= LevelUp)
            {
LevelUp: 
            NewLevel += + 1;
            LevelUp = LevelUp * 2 + 10;
            TotalSkillPoints += + SkillPoints;
            cout << "You leveled up to level " << NewLevel << " and gained " << SkillPoints << " skill points.\n";
            pause(5);
            cout << "You now have " << TotalSkillPoints << " unused skill points.\n";

            pause(5);
            }
            if(TotalExp >= LevelUp)
            {
            cout << "...\n";
            pause(1);
            goto LevelUp;
            }
            pause(5);
            system("CLS");
}
//------------------------------------------------------------------------------
int main(int argc, char *argv[])
{
     Name();
     Age();
     Gender();
     PlayerCard();
     replay:
     Program();
     YourType();
     OponentType();
     TypeAdvantages();//yet to return a value
     Rules();
     Battlephase();
     Earned();
     Level();
     cout << "Would you like to play again? y o n"<< endl;
     if(PlayAgain == "y")
     {
     goto replay;
     }
     system("PAUSE"); 
	 return 0;
}
void pause(int dur)
{
int temp = time(NULL) + dur;

while(temp > time(NULL));
}

[IceThatJaw]

I mentioned what your problem was in the other thread. If you declare variables inside the constructor then they are local that method. You don't need an explicit destructor unless you have dynamic data or you want to do run some extra code like logging when the object is destroyed so I took it out.

Try this:

struct VariableStorage
{
 
VariableStorage()
{
   Earnings = 10;
   Money = 0;
   AmountExp = 10;
   TotalExp = 0;
   NewLevel = 0;
   LevelUp = 10;
   SkillPoints = 3;
   TotalSkillPoints = 0;
}

string SetType;       
string PlayerAge;
string PlayerName;
string PlayerGender;
string CharacterDetails;
string OpponentCharacterDetails;
string Continue;
string PlayAgain;
string Type;
int Attack;
int Defense;
int Health;
string Advantage;
string Disadvantage;
string MonsterName;
int Damage;
string OType;
int OAttack;
int ODefense;
int OHealth;
string OAdvantage;
string ODisadvantage;
string OMonsterName;
int ODamage;
long Earnings;
long Money;
long AmountExp;
long TotalExp;
long NewLevel;
long LevelUp;
long SkillPoints;
long TotalSkillPoints;
int TypeOfCharacter;
};

[AbstractionAnon]

You have a good start with you Monster class, but there're a couple of problems.
1) In YourFireType(), YourWaterType(), YourNatureType() and likewise with the opponent functions, you have a memory leak. You dynamically allocate a Monster instance, but never delete it. In fact, you don't really do anything with it other that to set some values into it that are lost when the pointer to the new created instance goes out of scope.

2) After setting the values into the new Monster instance in each of those routines, you turn around and set the corresponding globals. There is no need for the globals type, attack, defense, health, advantage, disadvantage, monstername, nor the opponent versions of those. You already have that information in the respective Monster objects. You want to pass back the pointer to the monster you created and then deal with the two monsters.

Suggestion: Change each of your "yourtype" and "opponenttype" functions to return the pointer to the monster you created. For example:

Monster * YourFireType()
{	Monster * monster = new Monster;
	monster->SetType("fire");
	monster->SetAttack(8);
	monster->SetDefense(1);
	monster->SetHealth(40);
	monster->SetAdvantage("Nature");
	monster->SetDisadvantage("Water");
	monster->SetMonsterName("YourFireType");
	monster->DisplayStats();
	return monster;
}

You'll need to change the YourType and OpponentType to handle the returned pointer. Likewise, you'll need to change TypeAdvantages to compare the two monster instances.