C++ Prototype Function problems
Author
Hello,
I'm currently having a problem with a pretty basic and simple program. Basically, it's the start of a dungeons and dragons type of program, but even more basic than that, since I just rediscovered the joys programming.
http://rajen.joshi.googlepages.com/codingstuff
To save bandwith and make the code easier to read and comprehend, I uploaded it to my google pages. As you can see, this really is quite a primitive version of what I intend to create, since something like this is a good way to exercise my skills and develop further knowledge of C++.
Speaking in a very general manner, the program literally thus far should be able to do the following, but isn't doing all of it:
-Take in the user's name
-Introduce them to the "high-tech" navigation system of the game
-Give a basic intro to the story
-Jump from prototype function to prototype function until the end of the game is reached, and I have not programmed that far yet since I already have errors
The main problem lies within what the entire game relies on: the prototype functions after the main functions have ended. It literally should go from function to function. Perhaps it isn't the best way of doing this, and alternative methods are appreciated, but, to me, it does seem the simplest method at the moment with the small amount of c++ knowledge I possess.
Several different things have happened. First of all, randomly, or so it seems to me, the program decides to end after I input a number to perform some sort of action. Often, this happens when I input "0" as the variable "choice" to call up the statistics of my character. However, I also noticed that this is the last function that exists in each prototype function after the main has ended, and I am unsure of whether that factors into the problem.
In short, the program ends when it shouldn't be ending, and I'm very confused on how to proceed with this. Any ideas, thoughts, suggestions, comments, criticisms, solutions, or anything constructive would be appreciated!
Thanks!
The program quits when you input 0 because that's what you're telling it to do. When you input 0, after it displays your statistics, the function returs, which results in the program quitting. Just read through the code line by line, and you'll understand. What I'd recommend is that you define a function to take input. If it gets a zero, it displays statistics, and then loops back to get input again. If it gets a positive number, it returns it. Something like this:
So, it seems that besides the above problem, some of the prototype function calls are commented out, so they won't be executed, as shown above. Also, if you have multiple-line if statements, you need to put curly braces around all the ones you want to execute if the if statement is true. The above needs to be revised to:
or even
I recommend you look into switch statements and proper logic statement syntax (like for if statements). The way you're designing the program (a series of functions with options for where to go next) will probably work fine for a small game like this one - sort of a choose-your-own-adventure type thing.
//Get an integer value from the user, which can be up to the value of "max_choice"int GetInput(int max_choice){ int input = 0; do { std::cin >> input; if (input == 0) { //Display statistics } if (input > max_choice) std::cout << "Invalid option!\n"; } while (input == 0 || input > max_choice); //Loop until we get a non-zero value.} //Now, in one of your prototype functions:int field_n(){ cout << "You are in the north of the ghetto village. Where do you go?\n"; cout << "Options:\n 1.Tavern/Inn (get the local chatter)\n 2.The blacksmith\n 3.South Ghetto Village\n\n"; choice = GetInput(2); //2 options, so we pass GetInput a 2 if(choice == 1) //tavern(); //This line has been commented out! If choice == 1, the function returns and we quit the program. return 1; if(choice == 2) blacksmith(); return 1; return 1;}So, it seems that besides the above problem, some of the prototype function calls are commented out, so they won't be executed, as shown above. Also, if you have multiple-line if statements, you need to put curly braces around all the ones you want to execute if the if statement is true. The above needs to be revised to:
if (choice == 1){ tavern(); return 1;}else if (choice == 2){ blacksmith(); return 1;}or even
if (choice == 1) tavern(); //If there's only one line of code to be executed if the if statement is true, we don't need curly braceselse if (choice == 2) blacksmith();I recommend you look into switch statements and proper logic statement syntax (like for if statements). The way you're designing the program (a series of functions with options for where to go next) will probably work fine for a small game like this one - sort of a choose-your-own-adventure type thing.
if(choice == 1) //tavern(); return 1; if(choice == 2) blacksmith(); return 1; if(choice == 0) cout << "Attack is " << attack << "\n"; cout << "Defense is " << defense << "\n"; cout << "Level is " << lvl << "\n"; cout << "Weapon level is " << wep << "\n"; cout << "Money amount is " << rupee << " rupees\n\n"; return 1;A problem lies here in the if structure.... I gather by the way you are indenting that you want those indented statements to be within the if block. If you need two or more statements to be within in if block then you must use the { and }
if (condition){ statement 1; statement 2; . . statement n;}elseif (condition){ statement 1; statement 2; . . statement n;}else{ statement 1; statement 2; . . statement n;}I would recomend using a switch style like this...
switch(choice){ case 0: { cout << "Attack is " << attack << "\n"; cout << "Defense is " << defense << "\n"; cout << "Level is " << lvl << "\n"; cout << "Weapon level is " << wep << "\n"; cout << "Money amount is " << rupee << " rupees\n\n"; return 1; } case 1: { //tavern(); return 1; } case 2: { blacksmith(); return 1; } default: { cout << "You must enter choices 0-2\n\n"; return 1; }}The same thing would apply to your blacksmith and your yet to be tavern function.
The reason why your program terminates is that after your field_n() function the program just quits
A better way so it doesn't quit unless you wanted it to is to loop the field_n function until a certain condition is met
while (field_n());
You would have to put an option to quit by entering say -1 in your field_n function and have it return 0 rather than 1
Good luck :-)
I had another look over at your code and it would seem field_n relates to the north field.
You would have to code a little differently but you may get the idea if you follow my code
The code you have posted there is scarcely longer than your actual post ;\ In the future, please be aware that For Beginners is intended to handle basic problems such as this, and that you can post code within [code][/code] (short, typewriter-font blocks) or [source][/source] (for long snippets in iframes - you get automatic syntax highlighting, too) tags to preserve formatting.
Normally I would be quite harsh with you because you didn't post in FB like you were supposed to (and wherein you can expect gentle treatment). But I can see you *really* need help, so:
- The dice rolling function (a) doesn't prevent you from rolling a 0; (b) communicates the rolled value to the outside world via a global variable, which is silly (that's what return values are for); (c) forces the value to be printed (which you might not want to happen every time you roll the die); (d) calls srand() every time in spite of the comment (which I assume is part of some code you blithely copy-and-pasted from somewhere else) '//perform this only once!!!'. That's actually quite important, you want to do it once at the top of your *program*, not the top of that function.
- The language does *not* recognize your indentation as "setting off" if-blocks - some other languages do that, but not C++. You need to surround the "blocks" of things to do in each situation with curly braces ( {} ).
- Reading in integers interactively is tricky. I will show a robust solution below.
- The way that you call functions from each other isn't the way they're meant to work. Don't call functions for flow control; call them to delegate work. They don't mean "do this next"; they mean "do this *as a part of* what I'm doing". Otherwise, you just keep piling stuff on the stack. It will take a very long time for this to cause problems, but it's still very unelegant and demonstrates a serious lack of understanding.
What you want to do is have the return value for a "location" function encode "where to go next". Then, in main(), you have a loop where you continually call some location according to what the previous call returned.
- Along the same lines: always returning 1 doesn't really give you any information. If you don't need to return anything, you can use a 'void' return type. If you meant it to be a code indicating "I was successful", then (a) you should only do that for functions that could fail in some way; (b) you should not do that if there's a *more* appropriate return value; (c) you should normally use a 'bool' return type.
- You don't actually use string.h or math.h in your program, and you shouldn't use those in C++ anyway: the proper names for those C++ libraries are "cstring" and "cmath". However, "new" C++ programs (i.e. ones that don't have to interoperate with already-existing C programs or libraries) should basically never use "cstring", either, because the C++ standard library provides a much better option for string data, in "string". (I see you're using one std::string; OK, but you're not including the proper header for it; it only compiles by accident because you're getting it indirectly via iostream, which isn't guaranteed.) Oh, and yes, "time.h" is now "ctime", too.
- Explicitly returning EXIT_SUCCESS is a C-ism; in C++ you can just let main() reach the end, and it will implicitly return 0 (which indicates success, and this is a special case for main()). Also, you should NOT pause your program artificially at the end; instead, learn how to run console programs properly (from an already open console window, or via a batch script).
- You can prototype main also as just 'int main()'. I recommend doing this when you aren't going to actually *use* argc and argv.
- You don't need to create temporary variables for things that are just passed to functions and then not used again. You can (and should, in normal cases) just put in the expression as a function parameter.
- If you must compare the same variable, which is of an integral type, to multiple values, it is usual to use a 'switch' statement.
- Use functions to cut out redundant things (anything that you could have written mostly by copy-and-paste). In your case, that's the statistics display.
- On the topic of redundancy: Instead of "x = x + 10", you can write "x += 10".
- My personal recommendation is to avoid prototyping functions by implementing them up top (before their first use). That way, any circular dependencies are highlighted (because you will be forced to pick one or the other to prototype) and you save typing (and maintenance work).
Putting alllllll of that together, we get:
There are actually still a lot of problems, but I don't want to overwhelm you.
P.S. Zelda rocks :) "You gonna buy something or what?" (I think I have that right x.x)
Normally I would be quite harsh with you because you didn't post in FB like you were supposed to (and wherein you can expect gentle treatment). But I can see you *really* need help, so:
- The dice rolling function (a) doesn't prevent you from rolling a 0; (b) communicates the rolled value to the outside world via a global variable, which is silly (that's what return values are for); (c) forces the value to be printed (which you might not want to happen every time you roll the die); (d) calls srand() every time in spite of the comment (which I assume is part of some code you blithely copy-and-pasted from somewhere else) '//perform this only once!!!'. That's actually quite important, you want to do it once at the top of your *program*, not the top of that function.
- The language does *not* recognize your indentation as "setting off" if-blocks - some other languages do that, but not C++. You need to surround the "blocks" of things to do in each situation with curly braces ( {} ).
- Reading in integers interactively is tricky. I will show a robust solution below.
- The way that you call functions from each other isn't the way they're meant to work. Don't call functions for flow control; call them to delegate work. They don't mean "do this next"; they mean "do this *as a part of* what I'm doing". Otherwise, you just keep piling stuff on the stack. It will take a very long time for this to cause problems, but it's still very unelegant and demonstrates a serious lack of understanding.
What you want to do is have the return value for a "location" function encode "where to go next". Then, in main(), you have a loop where you continually call some location according to what the previous call returned.
- Along the same lines: always returning 1 doesn't really give you any information. If you don't need to return anything, you can use a 'void' return type. If you meant it to be a code indicating "I was successful", then (a) you should only do that for functions that could fail in some way; (b) you should not do that if there's a *more* appropriate return value; (c) you should normally use a 'bool' return type.
- You don't actually use string.h or math.h in your program, and you shouldn't use those in C++ anyway: the proper names for those C++ libraries are "cstring" and "cmath". However, "new" C++ programs (i.e. ones that don't have to interoperate with already-existing C programs or libraries) should basically never use "cstring", either, because the C++ standard library provides a much better option for string data, in "string". (I see you're using one std::string; OK, but you're not including the proper header for it; it only compiles by accident because you're getting it indirectly via iostream, which isn't guaranteed.) Oh, and yes, "time.h" is now "ctime", too.
- Explicitly returning EXIT_SUCCESS is a C-ism; in C++ you can just let main() reach the end, and it will implicitly return 0 (which indicates success, and this is a special case for main()). Also, you should NOT pause your program artificially at the end; instead, learn how to run console programs properly (from an already open console window, or via a batch script).
- You can prototype main also as just 'int main()'. I recommend doing this when you aren't going to actually *use* argc and argv.
- You don't need to create temporary variables for things that are just passed to functions and then not used again. You can (and should, in normal cases) just put in the expression as a function parameter.
- If you must compare the same variable, which is of an integral type, to multiple values, it is usual to use a 'switch' statement.
- Use functions to cut out redundant things (anything that you could have written mostly by copy-and-paste). In your case, that's the statistics display.
- On the topic of redundancy: Instead of "x = x + 10", you can write "x += 10".
- My personal recommendation is to avoid prototyping functions by implementing them up top (before their first use). That way, any circular dependencies are highlighted (because you will be forced to pick one or the other to prototype) and you save typing (and maintenance work).
Putting alllllll of that together, we get:
#include <cstdlib>#include <iostream>#include <ctime>#include <string>#include <sstream> // this will be used for the robust input I mentioned.using namespace std;//Declaring global variablesint attack = 1;int defense = 1;int exper = 0;int lvl = 1;int wep = 1;int rupee = 150;string name;//Roll a 10-sided die for various applicationsint roll_10() { return rand() % 10 + 1;}// Here is our function for inputting a number in a desired range.int promptForNumber(int lowestAccepted, int highestAccepted) { int result; string line; while (true) { getline(cin, line); if ((stringstream(line) >> result) && // we could re-interpret the line as a number result >= lowestAccepted && // AND it's big enough result <= highestAccepted) { // AND it's small enough return result; // breaks out of the entire function } // Otherwise, we show an error message and repeat the loop to get another // whole line of input. cout << "Sorry, I need a number between " << lowestAccepted << " and " << highestAccepted << ". Try again? "; }}void statistics() { cout << "Attack is " << attack << "\n"; cout << "Defense is " << defense << "\n"; cout << "Level is " << lvl << "\n"; cout << "Weapon level is " << wep << "\n"; cout << "Money amount is " << rupee << " rupees\n\n";}int field_n() { cout << "You are in the north of the ghetto village. Where do you go?\n" << "Options:\n 1.Tavern/Inn (get the local chatter)\n" << " 2.The blacksmith\n 3.South Ghetto Village\n" << endl; switch (promptForNumber(0, 3)) { // notice how there is no variable :) case 0: statistics(); return 0; // come back here (north field) case 1: return 3; // go to tavern case 2: return 2; // go to blacksmith case 3: return 1; // go to south field }}int blacksmith() { cout << "You can purchase a couple of items from the blacksmith:\n" << "1.Kokiri Sword(100 rupees)\n"; << "2.Goron Scale Armor(200 rupees)\n"; << "3.or exit back to the north ghetto village\n" << endl; switch (promptForNumber(0, 3)) { // notice how there is no variable :) case 0: statistics(); return 2; // come back here (blacksmith) case 1: cout << "You buy a kokiri sword, attack +1\n" << endl; attack += 1; // or just 'attack++' like you had, but I feel this way // is more expressive rupee -= 100; return 0; // go back to north field case 2: cout << "You buy Goron Scale Armor, defense +2\n" << endl; defense += 2; rupee -= 200; return 0; // go back to north field case 3: return 0; // don't buy anything, just go back }}int main() { srand(time(0)); // perform this only once for really real! cout << "What is your name?\n"; getline(cin, name); // grab the whole line, not just the first word, and don't // leave any text behind. cout << "Your name is: " << name << ".\n\n\n" << "You are in a small village in the land of Hyrule. You hate it\n" << "there because it's hella ghetto, so you decide to leave one day.\n" << "You must prepare for your adventure first though. Where do you go?\n" << "(use numbers only (1, 2, 3, etc.), or use 0 for your statistics at any time)\n" << endl; // yes, that's all one statement. // Now, here's our game loop, using the "next_room" and "choice" *properly*. // By the way, there's no reason they should be global. int room = 0; // the starting room is the north field. while (room != -1) { // we'll use that as a special value meaning the game is over switch (room) { // Each time, we call one of these functions, and thereby get the next // room value. We assign the value, and then "break", which leaves the // switch statement. That reaches the end of the "while", so we loop back // and do it again, as long as 'room' is not -1. case 0: room = field_n(); break; case 1: room = field_s(); break; case 2: room = blacksmith(); break; case 3: room = tavern_inn(); break; } } // When we get here, we are done.}There are actually still a lot of problems, but I don't want to overwhelm you.
P.S. Zelda rocks :) "You gonna buy something or what?" (I think I have that right x.x)
Author
Thanks everyone, my problems have been sorted out. Sorry for posting in the wrong section, I guess I was sort of in a rush yesterday because I was overloaded with homework, a feeling I'm sure many of you can relate to :/ .
Anyways, I did spend some time with your comments and what not, and it's making more sense now. I decided to reorganize the program a bit and rename some of the functions I have since I didn't completely understand the returning bit. A friend of mine who is in the same programming class as I am came to the library with me where we went through these suggestions and almost redesigned the entire thing. Now the program is running flawlessly thus far and we hope to have this finished soon. I'll post the final program when it is done in the for beginners section for any final suggestions and what not.
I haven't fully implemented everything you c++ walking dictionaries have been suggesting, since I haven't fully worked everything out, such as the rolling function, but I will definitely look into more reliable and powerful methods of accomplishing what needs to be done.
One more thing - and that is: Thank you. When I came back to this thread just a day later, I didn't expect to find this tremendous amount of constructive criticism. I was left speechless delving through what had been posted. Thanks guys, you are an extremely large helping of awesome. :) (and yes, zelda does rock)
Anyways, I did spend some time with your comments and what not, and it's making more sense now. I decided to reorganize the program a bit and rename some of the functions I have since I didn't completely understand the returning bit. A friend of mine who is in the same programming class as I am came to the library with me where we went through these suggestions and almost redesigned the entire thing. Now the program is running flawlessly thus far and we hope to have this finished soon. I'll post the final program when it is done in the for beginners section for any final suggestions and what not.
I haven't fully implemented everything you c++ walking dictionaries have been suggesting, since I haven't fully worked everything out, such as the rolling function, but I will definitely look into more reliable and powerful methods of accomplishing what needs to be done.
One more thing - and that is: Thank you. When I came back to this thread just a day later, I didn't expect to find this tremendous amount of constructive criticism. I was left speechless delving through what had been posted. Thanks guys, you are an extremely large helping of awesome. :) (and yes, zelda does rock)
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
