Jump to content
  • Advertisement
Sign in to follow this  
WiseElben

Are labels evil?

This topic is 4876 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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;
}

In every book or discussion, they always say that labels are evil. Well, when looking at this code, I could not find any easier way of doing it rather than using a label. Since labels are evil, what do I do now?! =)

Share this post


Link to post
Share on other sites
Advertisement
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...

Share this post


Link to post
Share on other sites
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

if (charInput != 'N' && charInput != 'n') {

// create file

if (fout.fail()) {
// notify failure
}

else {
// write data
}
}


Happy coding,
Pat.

Share this post


Link to post
Share on other sites
Quote:
Original post by darookie
Quote:
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
Quote:
Original post by darookie
Quote:
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.

Share this post


Link to post
Share on other sites
You could also break up the function - this sort of thing can be an indication that you're trying to do too much:


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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!