2
\$\begingroup\$

A few days ago I asked this question. So far I have made many changes in the code, as well as added some on and I was wondering if you see any way the code could be obviously optimized, as well as any suggestions you may have.

Note: I was thinking for the vampire race I would create a const variable called vampireFoodBar or something, and when something happens, either use ++ or --. It seems kinda hackish and sloppy, so is there any better way to do this?

#include <iostream>
#include <string>
#include <limits>
#include <fstream> 

//Function that creates pointer ">" Icon...Im lazy. 
void createIcon(void) { 
    std::cout << ">";
}

//Function clears cin
void cinClear(void) { 
    using namespace std;
    cin.clear();
    cin.ignore(numeric_limits<streamsize>::max(), '\n');
}


//Defines player class and attributes
class Player {
    public: 
        std::string name; 
        int age; 
        int gender;
        int race;
    };


int main(void) {


    Player user;

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

    std::cin >> user.name;

    //Asks for players gender 

    std::cout << "\nHello " << user.name << ".";

    //While loop to surround gender choice code, so if the user fucks up they can try again 
    while(true) { 
        std::cout << "\nAre you a boy or a girl?\n" 
        "1. Boy | 2. Girl\n" 
        "Choices are selected by typing in a single number\n";

        //Asks for players gender 
        int playerGenderSelect;
        createIcon();
        std::cin >> playerGenderSelect;

        //Sets gender depending on value entered. 1 being male, 2 being female. 
        switch(playerGenderSelect) { 

            case 1:
                user.gender = 1;
                break;

            case 2:
                user.gender = 2;
                break;

            default: 
                std::cout << "\nNot a valid number, try again\n";
                user.gender = 3;

            }

    //If the player entered a valid number, break loop, if not, continue it.
        if(user.gender == 1 or user.gender == 2) {
            break;
        }       

        else {
            cinClear();
            continue;
        } 


    }

    while(true) { 
        //Asks players for race. 
        int raceChoose; 
        std::cout << "\nPlease choose a race:\n"
                    "1. Elf\n"
                    "2. Human\n"
                    "3. Vampire\n"
                    "4. Druid\n"
                    "5. Troll\n"
                    "6. Orc\n"
                    "7. Dwarf\n"
                    "8. Race Info\n";


        std::cin >> raceChoose;         

        //Shit ton of writing. 

        std::string raceHelpElf = "Elf - The elves are an ancient race, older than all except the Dwarf.\n" 
                                "They are built on wisdom, and intelligence.\n"
                                "Many elves live in the wild, hunting their food with a sharp aim and a quick wit.\n"
                                "Elves have mastered the art of the bow, and are stealthier than any race known to Algoria\n"
                                "Elves are also jovial, light and happy by nature, throwing grand festivals day and night\n\n";

        std::string raceHelpHuman = "Human - The humans are the newest race to Algoria.\n"
                                    "They are known by other races to be greedy and power-hungry.\n"
                                    "Humans are the most ednuring creatures, and can survivie much.\n"
                                    "Humans are generally more equipped at fighting disease and poison.\n"
                                    "Humans are built on a race to power and control.\n\n"; 





        switch(raceChoose) {

            case 1:
                //Sets race to Elf
                user.race = 1;
                break;

            case 2:
                //Sets race to Human
                user.race = 2;
                break;

            case 3:
                //Sets race to Vampire
                user.race = 3;
                break;

            case 4: 
                //Sets race to Druid
                user.race = 4;
                break;

            case 5: 
                //Sets race to Troll
                user.race = 5;
                break;

            case 6: 
                //Sets race to Orc

                user.race = 6;
                break;

            case 7:
                //Sets race to Dwarf
                user.race = 7;
                break;
                //Sets race so it will print help. 
            case 8:
                user.race = 8; 
                break;

            default: 
                std::cout << "Not a vaild number, try again.";
                user.race = 0;

        }


            if(user.race >= 1 and user.race <= 10 and user.race != 8) {
                break;
            }

            else if(user.race == 8) {
                //Prints out all race help info 
                std::cout << 
                "\n\n" << 
                raceHelpElf <<
                raceHelpHuman <<
                raceHelpVampire; 
                continue; 


            }
            else { 
                //Clears cin and reruns loop
                using namespace std;
                cinClear();
                continue;
            }







    }








} 
\$\endgroup\$
  • \$\begingroup\$ Does "raceHelpVampire" refer to this? ;) \$\endgroup\$ – Hjulle Sep 24 '16 at 14:37
2
\$\begingroup\$

Use scoped enumerations

Modern C++ provides scoped enumerations:

enum class Gender {
    femail,
    male
};

enum class Race {
    elf,
    human,
    orc
};

Why not use them instead of integer constants 1, 2, 3, ...?

Data encapsulation

Also, I would suggest you keep the name, race, etc. fields as private, provide a constructor for Player, and the getters for getting all the four fields. This is not quite important in your code snippet, yet sometimes we need to maintain a particular class invariant via class methods, that check that the invariant is maintained. In such situation exporting the implementation might allow messing with those class invariants. (For example, a date class may not have a date with value 32.)

For instance:

class Player {
private:
    std::string name;
    int age;
    Gender gender;
    Race race;

public:
    Player(std::string& name_arg,
           int age_arg,
           Gender gender_arg,
           Race race_arg) :
    name    {name_arg},
    age     {age_arg},
    gender  {gender_arg},
    race    {race_arg}
    {}

    std::string& get_name() { return name; }
    int get_age()           { return age; }
    Gender get_gender()     { return gender; }
    Race get_race()         { return race; }
};

Hope that helps.

\$\endgroup\$
  • \$\begingroup\$ Okay, thank you for that. I had to research enums for a while, and now that I've started using them, they're MUCH easier to remember, \$\endgroup\$ – Alice The Hatter Sep 24 '16 at 17:33
  • \$\begingroup\$ @Alice Also, an IDE may provide code completion for enums: something that cannot be done for hardcoded enums. \$\endgroup\$ – coderodde Sep 24 '16 at 17:41
  • \$\begingroup\$ What exactly do you mean by code completion for the enums? \$\endgroup\$ – Alice The Hatter Sep 24 '16 at 18:36
  • \$\begingroup\$ @Alice an IDE (like XCode) will often give you the option to insert a phrase it recognizes. If you wrote "Hello World!" somewhere in your program, then the IDE may recommend "Hello" if it detects you beginning to write that word. \$\endgroup\$ – T145 Sep 24 '16 at 19:40
  • \$\begingroup\$ @Alice For example, when you write Gender::, an IDE may give you the list of all the Gender enumeration. Give it a try. \$\endgroup\$ – coderodde Sep 24 '16 at 19:50
3
\$\begingroup\$

The Nitpicking (from top to bottom)

  • You shouldn't need that createIcon function, and if you still want to keep it, why not rename it something more logical like printPointer. void should also not be needed as a parameter.
  • You don't need to comment code whose functionality is apparent. Comments are for code whose functionality is not readily apparent.
  • I'd add the following lines right after your include statements:

    using std::cin;
    using std::cout;
    using std::endl;
    using std::string;
    

    This will remove the need for the excessive std:: usage, as well as using namespace std;, which I shouldn't need to tell you doesn't belong anywhere in your code.

  • I would back up @coderodde's recommendations for what to do with your data structures. I'd add in that you should probably just rename the Player class to Creature (as I'm guessing you'll have friends and enemies), that has everything a character in the game would use. Any other races that have unique powers, like vampires, can be extended off of this class and be customized. This is the biggest benefit to using an OOP language.
  • Your main function doesn't require any parameters.
  • Instead of playerGenderSelect, I'd just say genderSelect. Later on in your code, you don't say playerRaceChoose, so this seems reasonable.
  • It's general OOP style to include spaces in your conditionals, like so:

    if (...) { ... }
    while (true) { ... }
    for (...) { ... }
    
  • Please format your code properly. The spacing is all over the place.
\$\endgroup\$
  • \$\begingroup\$ I have a question however, I know using namespace std is a bad practice at the beginning of programs...but in this case its isolated to that specific function, so whats the harm of putting it there? \$\endgroup\$ – Alice The Hatter Sep 24 '16 at 17:29
  • \$\begingroup\$ I have fixed everything else, thanks for all your help! \$\endgroup\$ – Alice The Hatter Sep 24 '16 at 17:33
  • \$\begingroup\$ @Alice the reason for not using using namespace std; in that method is the same reason as why you wouldn't include it at the top of your program. Adding it in a method just changes the scope of its influence. Glad I could be of assistance! Don't forget to actually pick answer btw :P \$\endgroup\$ – T145 Sep 24 '16 at 19:37

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.