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

Text-based adventure game with too many conditional statements

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

Problem

For a school project, I am creating a text-based adventure game in C++. I have shown my code to my teacher and he is stating my code is not entirely logical in sequence and that I should make my if statements inside functions because there are so many. I need help as to know how to make these changes.

Note that this is one function of the code, and not it's entirety.

void road() {
    int choice2;
    int choice3;
    int event1;
    system("cls");
    cout > choice2;
    cin.ignore();
    system("cls");

    if (choice2 == 1) {
        int decision1;
        cout > decision1;
        cin.ignore();
        system("cls");

            if (decision1 == 1) {
                cout > choice3;
        cin.ignore();
        system("cls");
            if (choice3 == 1) {
                cout > choice4;
            cin.ignore();
            system("cls");

                if (choice4 == 1){
                    Ending();
                }
                else if (choice4 == 2){
                    road();
                }

        }
        else if (random1 == 1) {
            cout << "Your character screams at the top of his lungs, " << endl;
            cout << "eventually your breath gives out and you die because of lack of oxygen." << endl;
            system("pause");
            gameover();
        }

    }
}

Solution

I have found a couple of things that could help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. I don't know that you've actually done that, but it's an alarmingly common thing for new C++ programmers to do.

Avoid the use of global variables

I see that name is used but it's neither declared within the function nor passed to it, implying a global variable. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable.

Eliminate unused variables

This code declares a variable event1 but then does nothing with it. Your compiler is smart enough to help you find this kind of problem if you know how to ask it to do so.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++.

Fix your newline characters

The string starting with "For some peculiar reason" contains /n which is two characters, but it's clear from the context that you intended \n which is a single newline character.

Use a menu object or at least a common menu function

In a number of places in your code, you have something like a menu. Your code presents a couple of options and then asks the user to pick one based on an input number. Rather than repeating that code in many places, it would make sense to make it generic. Only the prompt strings actually change, but the underlying logic of presenting the choices and asking for input are all the same. It looks like you're a beginning programmer, and so perhaps you haven't learned about objects yet, but this kind of repeated task with associated data is really well-suited to object-oriented programming and that's something that C++ is very good at expressing.

Specifically, here's how I might approach this. I'd create a Menu object:

class Menu
{
public:
    Menu(const string &name, const string &prompt, 
        const std::vector > &choices 
        = std::vector >{});
    virtual ~Menu();
    const string& getChoice() const;
    bool operator==(const string &name) const;
private:
    static const string menuend;
    string _name, _prompt;
    std::vector > _choices;
};


Implementations are here:

Menu::Menu(const string &name, const string &prompt, 
        const std::vector > &choices) 
    : _name(name), _prompt(prompt), _choices(choices) 
{}

bool Menu::operator==(const string &name) const
{
    return name==_name;
}

const string& Menu::getChoice() const
{ 
    if (_choices.size() == 0) {
        cout > choice; 
        --choice;
    } while (choice >= _choices.size()); 
    return _choices[choice].second; 
}

Menu::~Menu() 
{}

const string Menu::menuend{"END"};


Finally, we can construct the game itself as a std::vector of these Menu objects:

```
std::vector game{
Menu("mainroad",
"You are on a road that heads west and east of your position.\n"
"Which way will you go?\n", std::vector >{
{"Go West", "spider"},
{"Go East", "brickhouse"},
{"Wait for something to happen", "dragon"}}),
Menu("spider",
"You travel down the road, about only 100 metres and you encounter \n"
"a giant spider with vicious poison coated fangs.\n"
"its hideous appearance causes your throat to dry and your knees to shake!\n"
"What on earth will you do?\n\n", std::vector >{
{"Attempt to attack the spider with your sword.","spiderattack"},
{"Throw your sword in the off chance it might kill it.","throwsword"},
{"RUN FOR YOUR LIFE!", "running"}}),
Menu("spiderattack",
"You viscously swing your sword at the spiders general direction.\n"
"The swing was so great, your arms jolts out of place,\n"
"creating a surge of pain.\n"
"Your arm is now broken, and you fall to the ground in pain....\n"
"The spider launches 3 metres straight into your body...\n"
"What on earth is it doing?\n"
"Oh My God! The spider is devouring everything....\n"
"All that remained was bones of the once mobile adventurer.\n"),
Menu("brickhouse",
"After a mile walk, you arrive at an old brick house.\n"

Code Snippets

class Menu
{
public:
    Menu(const string &name, const string &prompt, 
        const std::vector<std::pair<string, string> > &choices 
        = std::vector<std::pair<string, string> >{});
    virtual ~Menu();
    const string& getChoice() const;
    bool operator==(const string &name) const;
private:
    static const string menuend;
    string _name, _prompt;
    std::vector<std::pair<string, string> > _choices;
};
Menu::Menu(const string &name, const string &prompt, 
        const std::vector<std::pair<string, string> > &choices) 
    : _name(name), _prompt(prompt), _choices(choices) 
{}

bool Menu::operator==(const string &name) const
{
    return name==_name;
}

const string& Menu::getChoice() const
{ 
    if (_choices.size() == 0) {
        cout << _prompt;
        return menuend;
    }
    unsigned choice; 
    int i;
    do { 
        cout << _prompt;
        i = 1;
        for (auto ch : _choices)
            cout << i++ << ": " << ch.first << '\n';
        cin >> choice; 
        --choice;
    } while (choice >= _choices.size()); 
    return _choices[choice].second; 
}

Menu::~Menu() 
{}

const string Menu::menuend{"END"};
std::vector<Menu> game{  
    Menu("mainroad", 
            "You are on a road that heads west and east of your position.\n" 
            "Which way will you go?\n", std::vector<std::pair<string,string> >{
                {"Go West", "spider"}, 
                {"Go East", "brickhouse"}, 
                {"Wait for something to happen", "dragon"}}),
    Menu("spider", 
            "You travel down the road, about only 100 metres and you encounter \n" 
            "a giant spider with vicious poison coated fangs.\n" 
            "its hideous appearance causes your throat to dry and your knees to shake!\n"
            "What on earth will you do?\n\n", std::vector<std::pair<string, string> >{
                {"Attempt to attack the spider with your sword.","spiderattack"},
                {"Throw your sword in the off chance it might kill it.","throwsword"},
                {"RUN FOR YOUR LIFE!", "running"}}),
    Menu("spiderattack",
            "You viscously swing your sword at the spiders general direction.\n" 
            "The swing was so great, your arms jolts out of place,\n"
            "creating a surge of pain.\n" 
            "Your arm is now broken, and you fall to the ground in pain....\n" 
            "The spider launches 3 metres straight into your body...\n"
            "What on earth is it doing?\n" 
            "Oh My God! The spider is devouring everything....\n" 
            "All that remained was bones of the once mobile adventurer.\n"), 
    Menu("brickhouse",
            "After a mile walk, you arrive at an old brick house.\n" 
            "You walk slowly inside.\n" 
            "The door slams behind you and the room lightens up.\n" 
            "What on earth is going on...?\n\n" 
            "Unable to open the door, you look around for anything of use.\n" 
            "Nothing, not a single piece of furniture.\n" 
            "What will you do?\n", std::vector<std::pair<string, string> >{
                {"Wait for someone to save you.", "trapdoor"}, 
                {"Or Wait for someone to save you.", "library"}})
};
void road() {
    auto menu = std::find(game.begin(), game.end(), "mainroad"); 
    while (menu != game.end())
        menu = std::find(game.begin(), game.end(), menu->getChoice());
}
random1 = rand() % 2;

Context

StackExchange Code Review Q#49614, answer score: 21

Revisions (0)

No revisions yet.