4
\$\begingroup\$
#include <iostream>
#include <string>
#define NL "\n"

int main(void) {

    //Acquires players first name 
    std::string introOne = "Welcome to The World of Magick\nWhat is your name?\n";
    std::cout << introOne;

    std::string playerName;
    std::cin >> playerName;

    //Asks for players gender 
    while(true) { 
    std::cout << "Are you a boy or a girl?" << NL << "1. Boy | 2. Girl" 
    << NL << "Choices are selected by typing in a single number" << NL;

    int playerGenderSelect;
    std::string playerGender;
    std::cin >> playerGenderSelect;


        //Sets gender depending on value entered
        switch(playerGenderSelect) { 

        case 1:
        playerGender = "male";
        break;

        case 2:
        playerGender = "female";
        break;

        default: 
        std::cout << "Not a valid number, try again" << NL;



    }

    //If the player entered a valid number, break loop, if not, continue it.
    if(playerGender == "male" or playerGender == "female") {
        break;
    }
    else { 
        continue;
    }
} 




    return 0;
} 

I wrote this code for a text based adventure game I'm making too keep me busy over the weekend.

I usually do this kind of boredom-project in python but decided to tackle it in C++. I was wondering if there were any obvious taboos or things that could obviously be optimized better.

\$\endgroup\$
  • \$\begingroup\$ any obvious taboos I can think of a few when you limit genders to binary male/female. \$\endgroup\$ – Jeroen Vannevel Sep 17 '16 at 19:18
  • \$\begingroup\$ I feel like this is a joke XD. If not there is a reason for that. \$\endgroup\$ – Alice The Hatter Sep 17 '16 at 19:22
  • 1
    \$\begingroup\$ It was, but it also wasn't. Most people will be just fine with male and female but others wouldn't (transexuals, hermaphrodites, moonkins, whatever). Whether you cater for that or not is up to you but given the phrasing "any obvious taboos" and the current drive to account for things like this (facebook has a 'custom' field for gender) I couldn't help but point it out. \$\endgroup\$ – Jeroen Vannevel Sep 17 '16 at 19:26
  • \$\begingroup\$ I understand, Im transgender myself and so I really did consider that, but later in the game based on the gender selection, pronouns would change. This would be hard to implement if you were say, gendervoid. Being their are so many nuances in the modern gender spectrum, I just feel like it would be a lot of work for a very small part of the population. As well as I might not even release this. If I was presented with this, I would say female. So..maybe I could find a way for doing it. But that was the reason for not including it. \$\endgroup\$ – Alice The Hatter Sep 17 '16 at 19:31
  • \$\begingroup\$ I suggest fixing your indentation... \$\endgroup\$ – Deduplicator Sep 17 '16 at 19:52
4
\$\begingroup\$

I see a number of things that may help you improve your code.

Use objects

The code has two things that look like a menu. It also collects two pieces of data about a single player. Those things, menu and player could and probably should be objects. Perhaps you're a beginning programmer, and 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. For an example of a usable menu object, see this answer.

Prefer const variables to #define

Instead of using the old C-style #define as in this line:

#define NL "\n"

It's generally better and more type-safe to use a const variable like this:

static const std::string NL{"\n"};

However, I'd probably not define this particular one in either case and simply type \n where it's needed.

Use constant string concatenation

The code currently contains this:

std::cout << "Are you a boy or a girl?" << NL << "1. Boy | 2. Girl" 
<< NL << "Choices are selected by typing in a single number" << NL;

But you don't really need to do it that way which potentially calls the << operator six times. Instead, you could express the same thing as this:

std::cout << "Are you a boy or a girl?\n" 
    "1. Boy | 2. Girl\n" 
    "Choices are selected by typing in a single number\n";

Which is both more readable and only calls << once. The compiler automatically concatenates the string literals together.

Use consistent formatting

The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.

Sanitize user input better

The code doesn't quite work as posted. If I enter a string such as "Edward" to answer a question, the program stays in an endless loop. It would be better to read a (text) line in and then convert it to a number. Users can do funny things and you want your program to be robust.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

\$\endgroup\$
  • \$\begingroup\$ Hey Edward, thanks a lot for this answer! Ive done basically everything you asked, and was very surprised than when doing what you said, such as entering a string like "Edward" on the second question, the loop was infinite. At this part im still confused as I wouldnt know how to implement what you said. You were correct in assuming I was a beginner, at least in C++. \$\endgroup\$ – Alice The Hatter Sep 18 '16 at 2:47
  • \$\begingroup\$ This question asks exactly that and got some answers. This question shows a couple of ways to address the issue. \$\endgroup\$ – Edward Sep 18 '16 at 3:14
  • \$\begingroup\$ Awesome! Thanks for that, I figured out what was going wrong. When they would put in a string, the cin stream still had that wrong input so it would continually read wrong input, once you flushed and cleared cin, it would just do it once. Thanks for all your help! \$\endgroup\$ – Alice The Hatter Sep 18 '16 at 3:26
  • \$\begingroup\$ Yes, you've got it! \$\endgroup\$ – Edward Sep 18 '16 at 13:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.