Sign in to follow this  
Levistus

Goto or Alternative

Recommended Posts

Levistus    152
Hi guys.I have a function in my practice rpg program that asks if the User is sure he wants to be the class he chose. Here's my chooseClass function.
void chooseClass(string heroName)
     {
           system("CLS");
           cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
           cout << "(a)Warrior" << endl;
           cout << "(b)Wizard" << endl;
           cout << "(c)Archer" << endl;
           
           char classChoice;
           do classChoice = getch();
           while (toupper(classChoice) != 'A' && toupper(classChoice) != 'B' && toupper(classChoice) != 'C');
           
           if (toupper(classChoice) == 'A')
           {
             cout << "You have chosen to be a Warrior. The master of melee fighting." << endl;
             cout << "Are you sure you want to be a Warrior? (Y)es or (N)o?" << endl;
             char choice;
             do choice = getch();
             while (toupper(choice) != 'Y' && toupper(choice) != 'N');
             if (toupper(choice) == 'Y')
             {
                 string heroClass = "Warrior";
                 Player Hero(heroName, heroClass, 100, 30, 15, 10);
                 stats(&Hero);
             }
             else
             {
                 chooseClass(heroName);
             }
           }
           
           else if (toupper(classChoice) == 'B')
           {
             cout << "You have chosen to be a Wizard. The master of magic." << endl;
             cout << "Are you sure you want to be a Wizard? (Y)es or (N)o?" << endl;
             char choice;
             do choice = getch();
             while (toupper(choice) != 'Y' && toupper(choice) != 'N');
             if (toupper(choice) == 'Y')
             { 
                 string heroClass = "Wizard";
                 Player Hero(heroName, heroClass, 50, 10, 10, 40);
                 stats(&Hero);
             }
             else
             {
                 chooseClass(heroName);
             }
           }
             else if (toupper(classChoice) == 'C')
           {
             cout << "You have chosen to be an Archer. The master of Archery(yeah, lame)." << endl;
             cout << "Are you sure you want to be an Archer? (Y)es or (N)o?" << endl;
             char choice;
             do choice = getch();
             while (toupper(choice) != 'Y' && toupper(choice) != 'N');
             if (toupper(choice) == 'Y')
             {
                 string heroClass = "Archer";
                 Player Hero(heroName, heroClass, 70, 15, 30, 20);
                 stats(&Hero);
             }
             else
             {
                 chooseClass(heroName);
             }
          }
            
     }

As you can see I can keep calling chooseClass non stop if I can't make my mind in choosing my class especially if I make more classes. I even checked the memory usage while I keep inputting 'N' and choosing a class again. It should rose. It should rise right? That's why I'm asking if I should use Goto here or is there a better a better alternative? Thank you guys in advance.

Share this post


Link to post
Share on other sites
Levistus    152
Quote:
Original post by mcmuzzle
have a look at this


Player *Hero = NULL

while (NULL == Hero)
{
//ask the class choice

//ask confirmation

//if confirmed then
// Hero = new Player (....);
}


Ok I tried it But I don't know if I placed it correctly. It didn't work though and it went to say "It didn't work" and asked for input.


void chooseClass(string heroName)
{
system("CLS");
cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
cout << "(a)Warrior" << endl;
cout << "(b)Wizard" << endl;
cout << "(c)Archer" << endl;

char classChoice;
do classChoice = getch();
while (toupper(classChoice) != 'A' && toupper(classChoice) != 'B' && toupper(classChoice) != 'C');

Player *Hero = NULL;
while (NULL == Hero)


if (toupper(classChoice) == 'A')
{
cout << "You have chosen to be a Warrior. The master of melee fighting." << endl;
cout << "Are you sure you want to be a Warrior? (Y)es or (N)o?" << endl;
char choice;
do choice = getch();
while (toupper(choice) != 'Y' && toupper(choice) != 'N');
if (toupper(choice) == 'Y')
{
string heroClass = "Warrior";
Player Hero(heroName, heroClass, 100, 30, 15, 10);
stats(&Hero);
}
else
{
//chooseClass(heroName);
cout << "It didn't work";
int v;
cin >>v;
}




Share this post


Link to post
Share on other sites
Zahlman    1682
First off, freeze, and step away from the Task Manager. You cannot get reliable information about your program's actual memory usage this way. The OS is doing a zillion complicated things under the hood and beyond your control. You are correct that recursively calling a function tends to consume memory, but in this case it will do so at an impossibly slow rate, and release it all once the user finally makes up his mind. (You can improve things somewhat by passing the string argument by const reference, as you normally should anyway.)

You can still structure things much, much more neatly, though, and 'goto' is definitely not called for here.

You already know about while loops. Loop until you know which character to create, and THEN create it. Inside the loop, we just present the information for the chosen character class, and ask for a prompt. When we get confirmation, we proceed past the loop, and use the chosen class information (which we will have to remember; so that variable will be scoped outside the while loop) to create the character.

And please, try to indent a little more consistently. As far as I can tell, your current convention is to indent 5 spaces for the first level of indentation, 6 for the next, 2 for the next and 4 for the next after that. This is very difficult to follow.

Finally, keep in mind that in order to *do* anything with the created Player object, it will have to get passed back to the rest of the code. The natural way to do this is with the return value.

So, as a first step, we get this:


Player chooseClass(const string& heroName) {
char choice = 'N';
char classChoice;
while (choice != 'Y') {
system("CLS");
cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
cout << "(a)Warrior" << endl;
cout << "(b)Wizard" << endl;
cout << "(c)Archer" << endl;

do classChoice = getch();
while (toupper(classChoice) != 'A' && toupper(classChoice) != 'B' && toupper(classChoice) != 'C');

if (toupper(classChoice) == 'A') {
cout << "You have chosen to be a Warrior. The master of melee fighting." << endl;
cout << "Are you sure you want to be a Warrior? (Y)es or (N)o?" << endl;
} else if (toupper(classChoice) == 'B') {
cout << "You have chosen to be a Wizard. The master of magic." << endl;
cout << "Are you sure you want to be a Wizard? (Y)es or (N)o?" << endl;
} else { // must be 'C' for Archer
cout << "You have chosen to be an Archer. The master of Archery(yeah, lame)." << endl;
cout << "Are you sure you want to be an Archer? (Y)es or (N)o?" << endl;
}

do choice = getch();
while (toupper(choice) != 'Y' && toupper(choice) != 'N');
}

if (toupper(classChoice) == 'A') {
string heroClass = "Warrior";
Player Hero(heroName, heroClass, 100, 30, 15, 10);
stats(&Hero);
return Hero;
} else if (toupper(classChoice) == 'B'){
string heroClass = "Wizard";
Player Hero(heroName, heroClass, 50, 10, 10, 40);
stats(&Hero);
return Hero;
} else { // must be 'C' for Archer
string heroClass = "Archer";
Player Hero(heroName, heroClass, 70, 15, 30, 20);
stats(&Hero);
return Hero;
}
}



You'll notice that this restructuring has also made things a little shorter, by eliminating the repetition of the recursive calls.

Can we do better? Of course. Another side effect of restructuring things this way is that other redundant, "patterned" code has been moved closer together, making it easier to see what to change.

One thing that strikes me is that the messages given to confirm each class are very similar; we can substitute the class name (which is also used in the initial prompt!) and a brief "the master of" description, and reuse the same "template" to handle each case. (This is of course a template in the ordinary English sense, not the C++ one!)

So let's do just that. We'll want to store the optional bits of text in some kind of container, so we can retrieve the corresponding one according to the user's selection. We can map a letter into an array index very neatly, if we note that 'A', 'B' and 'C' are adjacent in ASCII, by simply subtracting 'A' from the uppercase version of the user's response. (Remember, chars are numbers.)

Similarly, we can store attributes for the various classes in a table, and use them when we make the constructor call.


Player chooseClass(const string& heroName) {
char choice = 'N';
char classChoice;
string classNames[3] = { "Warrior", "Wizard", "Archer" };

while (choice != 'Y') {
system("CLS");

cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
for (int i = 0; i < 3; ++i) {
// We can also use "char arithmetic" for labelling the menu, as long as
// the sum is cast back to char so that it prints as a character instead
// of a number.
cout << "(" << static_cast<char>('a' + i) << ")" << classNames[i] << endl;
}

do classChoice = getch();
while (toupper(classChoice) != 'A' && toupper(classChoice) != 'B' && toupper(classChoice) != 'C');

string classDescriptions[3] = {
"The master of melee fighting.",
"The master of magic."
"The master of Archery(yeah, lame)."
};

int classType = toupper(classChoice) - 'A';
cout << "You have chosen to be a " << classNames[classType] << ". " << classDescriptions[classType] << endl;
cout << "Are you sure you want to be a " << classNames[classType] << "? (Y)es or (N)o?" << endl;

do choice = getch();
while (toupper(choice) != 'Y' && toupper(choice) != 'N');
}

int heroStats[3][4] = {
{100, 30, 15, 10},
{50, 10, 10, 40},
{70, 15, 30, 20}
};

int classType = toupper(classChoice) - 'A';
string heroClass = classNames[classType];
Player Hero(heroName, heroClass, heroStats[classType][0], heroStats[classType][1], heroStats[classType][2], heroStats[classType][3]);
stats(&Hero);
return Hero;
}



Can we do better? Of course!

1) We're only ever interested in the result of toupper() applied to the getch() results, so it makes sense to call toupper() right away when we store the value, instead of repeating it everywhere.
2) The work that is done by the 'stats' function ought to be done by the Hero constructor, so move it there instead.


Player chooseClass(const string& heroName) {
char choice = 'N';
char classChoice;
string classNames[3] = { "Warrior", "Wizard", "Archer" };

while (choice != 'Y') {
system("CLS");

cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
for (int i = 0; i < 3; ++i) {
// We can also use "char arithmetic" for labelling the menu, as long as
// the sum is cast back to char so that it prints as a character instead
// of a number.
cout << "(" << static_cast<char>('a' + i) << ")" << classNames[i] << endl;
}

do classChoice = toupper(getch());
while (classChoice != 'A' && classChoice != 'B' && classChoice != 'C');

string classDescriptions[3] = {
"The master of melee fighting.",
"The master of magic."
"The master of Archery(yeah, lame)."
};

int classType = toupper(classChoice) - 'A';
cout << "You have chosen to be a " << classNames[classType] << ". " << classDescriptions[classType] << endl;
cout << "Are you sure you want to be a " << classNames[classType] << "? (Y)es or (N)o?" << endl;

do choice = toupper(getch());
while (choice) != 'Y' && choice != 'N');
}

int stats[3][4] = {
{100, 30, 15, 10},
{50, 10, 10, 40},
{70, 15, 30, 20}
};

int classType = classChoice - 'A';
string heroClass = classNames[classType];
return Player(heroName, heroClass, stats[classType][0], stats[classType][1], stats[classType][2], stats[classType][3]); // assuming the old 'stats' work is moved to the Player constructor.
}



Finally, we can observe that there are two separate places where we use a do-while loop with getch() to get input, and see if there's a way to wrap up that logic. This becomes especially important because the response for classChoice has to get "translated" in two separate spots, when we could translate it as part of the wrapper process.

We'll make a function that accepts a string of allowed 1-character uppercase responses, and returns a number (index into the string) to indicate which was picked, when it happens.


int getCharFromUser(const string& options) {
int index = string::npos;
while (index == string::npos) {
index = options.find(toupper(getch()));
}
return index;
} // This will probably be useful lots of other places :)

Player chooseClass(const string& heroName) {
// We'll no longer need a temporary variable here for "choice", because
// getCharFromUser() already handles that. Similarly, the chosen class type
// will be directly remembered as a number.
int classType;
string classNames[3] = { "Warrior", "Wizard", "Archer" };

do {
system("CLS");

cout << "Hello " << heroName << endl << "Which class do you want to be?" << endl;
for (int i = 0; i < 3; ++i) {
cout << "(" << static_cast<char>('a' + i) << ")" << classNames[i] << endl;
}

// 'A' -> 0, 'B' -> 1, 'C' -> 2 - just as before. No more char arithmetic
// for this part, though.
classType = getCharFromUser("ABC");

string classDescriptions[3] = {
"The master of melee fighting.",
"The master of magic."
"The master of Archery(yeah, lame)."
};

cout << "You have chosen to be a " << classNames[classType] << ". " << classDescriptions[classType] << endl;
cout << "Are you sure you want to be a " << classNames[classType] << "? (Y)es or (N)o?" << endl;
} while (getCharFromUser("YN") != 0);

int stats[3][4] = {
{100, 30, 15, 10},
{50, 10, 10, 40},
{70, 15, 30, 20}
};

// One other simplification: get a temp pointer to the appropriate row of
// the table.
int (*heroStats)[4] = stats[classType]; // pretty sure that's right :)

return Player(heroName, classNames[classType], heroStats[0], heroStats[1], heroStats[2], heroStats[3]);
}

Share this post


Link to post
Share on other sites
Levistus    152
Wow Zahlman that's a lot of stuff I got to learn. But first I want it to compile. I'm getting some errors. Here they are:

1. C:\Dev-Cpp\Functions.cpp In function `Player chooseClass(const std::string&)':
2. C:\Dev-Cpp\Functions.cpp cannot convert `int*' to `int (*)[4]' in initialization

Here's the Code they're referring to:
int (*heroStats)[4] = stats[classType]; // pretty sure that's right :)

3. C:\Dev-Cpp\Functions.cpp invalid conversion from `int*' to `int'
4. C:\Dev-Cpp\Functions.cpp initializing argument 3 of `Player::Player(std::string, std::string, int, int, int, int)'
and lots lots more of the same kind.

Here's the code they're referring to:
return Player(heroName, classNames[classType], heroStats[0], heroStats[1], heroStats[2], heroStats[3]);

This is great now I'll be using new stuff thanks to you. Arrays, that npos thingy(lol), static_cast and more. After this get's compiled I'm gonna study your post from start to finish. Thank you again Zahlman you always help me.

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by Levistus
Wow Zahlman that's a lot of stuff I got to learn. But first I want it to compile. I'm getting some errors. Here they are:

1. C:\Dev-Cpp\Functions.cpp In function `Player chooseClass(const std::string&)':
2. C:\Dev-Cpp\Functions.cpp cannot convert `int*' to `int (*)[4]' in initialization

Here's the Code they're referring to:
int (*heroStats)[4] = stats[classType]; // pretty sure that's right :)


Yeah, I guess I was wrong. :)

I guess it should be int (*heroStats)[4] = stats + classType;, or equivalently, int (*heroStats)[4] = &(stats[classType]); (and actually I'm not so sure about the second, although in general you can normally make that kind of equivalence).

More info.

After that, the rest should go away too.

Quote:
that npos thingy(lol)


:/
std::string::npos is the value returned by std::string's search methods to indicate "not found".

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this