HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Assigning stats in a game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamestatsassigning

Problem

I made a program that allows you to assign stats. However it uses some fairly complex if-else statements. Also, someone told me that goto statements were bad, but why is that? I know that switch might have been a better solution, but I cannot get switch statements to work for string variables.

#include 

int main()
{
    using namespace std;
    int strength,agility,perception,endurance,intelligence,rem_statpoints;
    cout > inc;
     if (inc == "strength" or "agility" or "perception" or "endurance" or "intelligence") {
         goto increase;
     } else {
         cout > ass_statpoints;
         if (ass_statpoints > rem_statpoints) {
             cout > red;
    string statreset;       //string statreset is placed here because I could not place it next to the relevant cin for some reason.
    if (red == "play!") {
        goto game;
    } else {
        cout > statreset;   //string statreset is placed here
        if (statreset == "yes") {
            cout > statresetno;
            if (statresetno == "yes") {
                goto game;
            } else {
                cout << "Make up you mind!" << endl;
                goto red;
            }
        }
    }
    goto stat;

game:{}
     {
         cout << "Initializing game" << endl;
         cout << "game coming soon!" << endl;
         cout << "Thank you for playing!" << endl;
     }
     return 0;
}

/* Some one told me once that is was bad to use alot of goto statements,
 * however, I can not see what damage they are causing here.
 * Then again, I am not familiar with any alternatives for goto and labels,
 * so I am forced to use goto!
 * I really hoped you enjoyed using my program, and if this program for
 * some reason helped someone else, I am glad.
 * Feel free to use this code for yourself if you really found it that
 * good
 *
 *
 * Happy Trails!
 * - Lemonizer
 */

Solution

Please put one variable per line:

int strength,agility,perception,endurance,intelligence,rem_statpoints;


That is really hard to read:

It would be even better to wrap this in a structure (one day you may have 2 players at the same time).

struct Player
{
    int strength;
    int agility;
    int perception;
    int endurance;
    int intelligence;
};
int rem_statpoints; // not part of a player.
                    // as it is just used during player creation


Would it not be easier to make this a function/method:

statreset:{}
          strength = 5;               //Strength     is set to 5 by default
          agility = 5;                //Agility      is set to 5 by default
          perception = 5;             //Perception   is set to 5 by default
          endurance = 5;              //Endurance    is set to 5 by default
          intelligence = 5;           //Intelligence is set to 5 by default
          rem_statpoints = 10;        //The avaivable number of stat points

void Player::statreset(int& rem_statpoints)
{
          strength = 5;               //Strength     is set to 5 by default
          agility = 5;                //Agility      is set to 5 by default
          perception = 5;             //Perception   is set to 5 by default
          endurance = 5;              //Endurance    is set to 5 by default
          intelligence = 5;           //Intelligence is set to 5 by default
          rem_statpoints = 10;        //The avaivable number of stat points
}


This looks like another mehtod to me:

stat:{}
     cout << "Strength       " << strength << endl;
     cout << "Agility        " << agility << endl;
     cout << "Perception     " << perception << endl;
     cout << "Endurance      " << endurance << endl;
     cout << "Intelligence   " << intelligence << "\n" << endl;
     cout << "What stat do you want to increase?" << endl;


This is not doing what you think it is doing.

I am surprised if this even works as it should always return true.

string inc;
     cin >> inc;
     if (inc == "strength" or "agility" or "perception" or "endurance" or "intelligence")

     // what you really want is:
     if (inc == "strength" || inc == "agility" || inc == "perception" ... etc


The comment at the end:

/* Some one told me once that is was bad to use alot of goto statements,
 * however, I can not see what damage they are causing here.


I can.
You are tightly binding your control flow logic.

This makes it hard to introduce new steps or alter the logic.

Following the logic in this code is even worse. It is the perfect example of spaghetti. If it works fine then great. But try following the logic when something breaks. This becomes a maintenance nightmare.

* Then again, I am not familiar with any alternatives for goto and labels,
 * so I am forced to use goto!


Alternatives:

Combine the following:

if () {} else {}
 for(;;) {}
 while() {}
 do {} while()

 ()


You can reduce the complexity of you of tree like this:

if ()
 {
 }
 else if ()
 {
 }
 else if ()
 {
 }
 else if ()
 {
 }
 // ...etc
 else
 {
 }


I would restructure the code like this:

int main()
 {
     Player  player1;
     do
     {
         generatePlayer(player1);
     }
     while(!isUserSatisfied());

     playGame();
 }


Then generatePlayer()

void generatePlayer(Player& player)
 {
     int extraPoints;

     player.statreset(extraPoints);
     while(extraPoints > 0)
     {
         std::cout > inc;
         }
         while(!isValidAttribute(inc));

         do
         {
              std::cout > amount;
         }
         while(amount > extraPoints);

         player.modifyAttribute(inc, amount);
     }
}

Code Snippets

int strength,agility,perception,endurance,intelligence,rem_statpoints;
struct Player
{
    int strength;
    int agility;
    int perception;
    int endurance;
    int intelligence;
};
int rem_statpoints; // not part of a player.
                    // as it is just used during player creation
statreset:{}
          strength = 5;               //Strength     is set to 5 by default
          agility = 5;                //Agility      is set to 5 by default
          perception = 5;             //Perception   is set to 5 by default
          endurance = 5;              //Endurance    is set to 5 by default
          intelligence = 5;           //Intelligence is set to 5 by default
          rem_statpoints = 10;        //The avaivable number of stat points

void Player::statreset(int& rem_statpoints)
{
          strength = 5;               //Strength     is set to 5 by default
          agility = 5;                //Agility      is set to 5 by default
          perception = 5;             //Perception   is set to 5 by default
          endurance = 5;              //Endurance    is set to 5 by default
          intelligence = 5;           //Intelligence is set to 5 by default
          rem_statpoints = 10;        //The avaivable number of stat points
}
stat:{}
     cout << "Strength       " << strength << endl;
     cout << "Agility        " << agility << endl;
     cout << "Perception     " << perception << endl;
     cout << "Endurance      " << endurance << endl;
     cout << "Intelligence   " << intelligence << "\n" << endl;
     cout << "What stat do you want to increase?" << endl;
string inc;
     cin >> inc;
     if (inc == "strength" or "agility" or "perception" or "endurance" or "intelligence")

     // what you really want is:
     if (inc == "strength" || inc == "agility" || inc == "perception" ... etc

Context

StackExchange Code Review Q#23372, answer score: 8

Revisions (0)

No revisions yet.