void saveHero()
{
cout << "Tired, " << hero.getName() << " walks to Save Lake to drink some of it's refreshing water..." << endl;
cout << "Save to Slot (1-3):";
cin >> input;
playCursorSound();
if(input > 3 || input < 0)
input = 3;
cout << "Are you sure you want to save to slot " << input << "? (Y or N): ";
cin >> charInput;
if(charInput == 'N' || charInput == 'n')
goto End;
switch(input)
{
case 1:
fout.open("hero1.txt");
break;
case 2:
fout.open("hero2.txt");
break;
default:
fout.open("hero3.txt");
break;
}
if(fout.fail())
{
cout << "\nERROR: Could not open file!" << endl;
goto End;
}
fout << "Name: " << hero.getName() << endl;
fout << "Class: " << hero.getClass() << endl;
fout << hero.getDesc() << endl;
fout << "MaxHP: " << hero.getMaxHP() << endl;
fout << "MaxMP: " << hero.getMaxMP() << endl;
fout << "HP: " << hero.getHP() << endl;
fout << "MP: " << hero.getMP() << endl;
fout << "Str: " << hero.getStr() << endl;
fout << "Def: " << hero.getDef() << endl;
fout << "Int: " << hero.getInt() << endl;
fout << "Dex: " << hero.getDex() << endl;
fout << "Gold: " << hero.getGold() << endl;
fout << "Helm: " << hero.getHelm() << endl;
fout << "Armor: " << hero.getArmor() << endl;
fout << "Weapon: " << hero.getWeapon() << endl;
fout << "Extra: " << hero.getExtra() << endl;
fout << "Items: 0 " << hero.getItemPotion() << " 1 "
<< hero.getItemSuperPotion << " 2 " << hero.getItemElixer()
<< " 3 " << hero.getItemSuperElixer()
<< " 4 " << hero.getItemBeserkPotion()
<< " 5 " << hero.getItemMagicGem() << endl;
fout.close();
cout << "The Legendary " << hero.getName() << " Saved." << endl;
End:
cout << endl;
}
Are labels evil?
Author
if(fout.fail()){ cout << "\nERROR: Could not open file!" << endl; return;}if(fout.fail()){ throw std::exception("ERROR: Could not open file!");}...
But no, goto aren't really evil. It's just that using one must be a deliberate decision, i.e. when the alternative to using one makes the code awkward, hard to understand...
Author
Oh wow, that was simple... I've been trying return 0; instead of return;. Silly me.
Thanks for the help. =)
Thanks for the help. =)
I can see only one valid reason for using gotos (e.g. labels) in C(++): breaking from nested loops across several levels. It's the most efficient way to achieve that. In most (if not any) other cases you can simply replace the construct with an if-else expression (or a break/continue in case of loops) without performance loss.
The problem with using gotos is not the construct itself, but improper/unnecessary or over-use of it.
The example you provided is not a good use of a goto. Aside from the fact that the condition is incorrect (e.g. any input other than 'n' leads to save while the text clearly states that only 'y' and 'n' are acceptable inputs[wink]), it can easily be replaced by
Happy coding,
Pat.
The problem with using gotos is not the construct itself, but improper/unnecessary or over-use of it.
The example you provided is not a good use of a goto. Aside from the fact that the condition is incorrect (e.g. any input other than 'n' leads to save while the text clearly states that only 'y' and 'n' are acceptable inputs[wink]), it can easily be replaced by
if (charInput != 'N' && charInput != 'n') { // create file if (fout.fail()) { // notify failure } else { // write data }}Happy coding,
Pat.
Quote:Original post by Sneftel
Why not just return?
Even better yet. *Turning of SFEP-advocate-mode*[wink]
Quote:Original post by darookieQuote:Original post by Sneftel
Why not just return?
Even better yet. *Turning of SFEP-advocate-mode*[wink]
Advocating single entry-point makes sense. Single exit-point does not.
Thankfully, C++ does not allow you to goto into a function.
Quote:Original post by FrunyQuote:Original post by darookieQuote:Original post by Sneftel
Why not just return?
Even better yet. *Turning of SFEP-advocate-mode*[wink]
Advocating single entry-point makes sense. Single exit-point does not.
Thankfully, C++ does not allow you to goto into a function.
I will not argue against that. Generally SFEP messes complex code up and leads to no good. For trivial functions I prefer it, though.
You could also break up the function - this sort of thing can be an indication that you're trying to do too much:
P.S. I get the general impression that you are using member variables for things that ought to be more restricted in scope (i.e. locals). This is nearly as bad as using globals for the same purpose. It's confusing because it implies there's a reason for the extra scope when there isn't.
void saveHero() { int slot = promptForSlot(); if (slot && saveToSlot(slot)) { cout << "The Legendary " << hero.getName() << " Saved." << endl; } cout << endl;}int promptForSlot() { int slot; char yesno; cout << "Tired, " << hero.getName() << " walks to Save Lake to drink some of it's refreshing water..." << endl; cout << "Save to Slot (1-3):"; cin >> slot; playCursorSound(); if (slot > 3 || slot < 0) slot = 3; cout << "Are you sure you want to save to slot " << slot << "? (Y or N): "; cin >> yesno; if (yesno == 'N' || yesno == 'n') slot = 0; return slot;}bool saveToSlot(int slot) { ifstream fout((stringstream("hero") << slot << ".txt").str().c_str()); bool fileFound = fout.good(); if(fileFound) { // Do all the file writing here... BTW, you may want to look into better // techniques for this } else { cout << "\nERROR: Could not open file!" << endl; } fout.close(); return fileFound;}P.S. I get the general impression that you are using member variables for things that ought to be more restricted in scope (i.e. locals). This is nearly as bad as using globals for the same purpose. It's confusing because it implies there's a reason for the extra scope when there isn't.
SFEP might be a good in pure C, but in C++ it not going to save you from things like exceptions. On the other hand, if you wrap up things like pointers in objects that will clean things up in their destructor (hint: std::auto_ptr) you can return anywhere you want, be exception safe, and write less code, all the the same time.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
