Guraiya
Guraiya

Reputation: 33

C++ How to translate my code to OOP with classes and functions?

I am quite new to OOP programming and would like to know how to get my code cleaner. The program works but I don't know how to make different classes for Genres, player input etc.

I tried creating a class for the Genres with a .h and .cpp file, but I still have not many knowledge on how to make my code cleaner, since everything is created in the: int main () {}

bookPicker.cpp

int main()
{
    //Declaring a user account
    std::string name, lastName;

    //Genre's
    std::string ice, fire, earth, wind;
    ice = "Ice", fire = "Fire", earth = "Earth", wind = "Wind";

    //Titles
    int a, b, c, d;


    //Recommendation
    std::string r;
    r = "Type yess if this is the genre of your choice\n";

    std::string iceS[4] = { "The Ice Gauntlet", "The Formal Ice Queen", "Frozen in Time", "Frost Lake" };
    std::string fireS[4] = { "The Fire Gauntlet", "The Formal Fire Queen", "Hot Air", "Fire Lake" };
    std::string earthS[4] = { "The Earth Gauntlet", "The Formal Earth Queen", "Stuck in Time", "The Swamp" };
    std::string windS[4] = { "The Wind Gauntlet", "The Formal Wind Queen", "Blown in Time", "Wind Lake" };    

    //Welcome
    std::string w, wU;
    w = "Welcome ";
    wU = " to The Four Elemets Book Store!\n";

    //Creating user account
    std::cout << "Please enter your name" << std::endl;
    std::cin >> name;
    std::cout << "Please enter your lastname" << std::endl;
    std::cin >> lastName;

    std::string userAccount = name + lastName;

    //Ask for input
    std::cout << w << userAccount << std::endl;
    std::cout << "What kind of genre do you like to read? \n" << std::endl;
    std::cout << ice << "\n" << fire << "\n" << earth << "\n" << wind << "\n" << std::endl;
    std::cout << "Please pick your genre\n" << std::endl;

    //create the choice string variable
    std::string choice;
    std::cin >> choice;

    //if statement after the input

        if (choice == ice) {
            std::cout << r << std::endl;
            std::cin >> a;
            std::cout << "\n";

            for (int i = 0; i < 4; i++) {
                std::cout << iceS[i] << "\n";
            }
        } if (choice == fire) {
            std::cout << r << std::endl;
            std::cin >> b;
            std::cout << "\n";

            for (int y = 0; y < 4; y++) {
                std::cout << fireS[y] << "\n";
            }
        } if (choice == earth) {
            std::cout << r << std::endl;
            std::cin >> c;
            std::cout << "\n";

            for (int x = 0; x < 4; x++) {
                std::cout << earthS[x] << "\n";
            }
        } if (choice == wind) {
            std::cout << r << std::endl;
            std::cin >> d;
            std::cout << "\n";

            for (int o = 0; o < 4; o++) {
                std::cout << windS[o] << "\n";
            }
        }



    return 0;

} 

Upvotes: 0

Views: 161

Answers (2)

Blaze
Blaze

Reputation: 16876

As the sample program is now, there's not much that OOP would improve. Of course, depending on how the program would evolve, that can change. If I would make a class, it would probably be this one:

class account {
public:
    std::string name;
    std::string lastName;

    std::string getFullName() {
        return name + " " + lastName;
    }
};

Here, account now represents a user record, and getFullName can be used for a clean print of their name. You make the account like this:

//Declaring a user account
account userAcc;

You set the names like this:

std::cout << "Please enter your name" << std::endl;
std::cin >> userAcc.name;
std::cout << "Please enter your lastname" << std::endl;
std::cin >> userAcc.lastName;

And then you print the name this way:

//Ask for input
std::cout << w << userAcc.getFullName() << std::endl;

Of course, this isn't going to improve anything as-is because you only have one user anyway. I see bigger potential for improvement in the repetitive if chain that make up about half of your code. Instead, I would leverage the fact that your four elements are used to enumerate things this way:

enum element {
    ICE, FIRE, EARTH, WIND, NONE
};

std::vector<std::vector<std::string>> books = {
    {"The Ice Gauntlet", "The Formal Ice Queen", "Frozen in Time", "Frost Lake" },
    { "The Fire Gauntlet", "The Formal Fire Queen", "Hot Air", "Fire Lake" },
    { "The Earth Gauntlet", "The Formal Earth Queen", "Stuck in Time", "The Swamp" },
    { "The Wind Gauntlet", "The Formal Wind Queen", "Blown in Time", "Wind Lake" }
};

Now we have an element enumeration, but more on that later. Note how the books are now a vector of vector of strings, a structure that is a lot like a table. Now to find out which element the user wants, we can query it like this:

//if statement after the input
element chosenElement = NONE;

if (choice == ice) chosenElement = ICE;
else if (choice == fire) chosenElement = FIRE;
else if (choice == earth) chosenElement = EARTH;
else if (choice == wind) chosenElement = WIND;

Now chosenElement represents the element that the user wanted, respectively NONE if he entered something invalid. Now you can print all the books this way:

if (chosenElement == NONE) {
    std::cout << "Sorry, we don't have this genre." << std::endl;
}
else for (std::string i : books[chosenElement]) {
    std::cout << i << std::endl;
}

This replaces the entire if chain that you had before. books[chosenElement] represents all the books in a genre (so books[ICE] is all the ice books, and so on) and i is the respective book, and they all get printed this way.

The program can also be extended more easily that way. Suppose you have a new element, all you had to do is add its value to the enum definition before the NONE, add the row of books to the books vector of vectors of strings, add an else if (choice... check for the new element, and you're done. No need to copy/paste any if block. Also, you're not dependent on hard-coding the amount of books per genre anymore. You could just add a fifth book to a genre and it would work like that. No need to tell it how many books there are in the genre because the range-based for loop find that out on its own.

Also, if you're only going to need a string once and won't modify it, I would suggest not making an std::string for it. Instead of this:

std::string ice, fire, earth, wind;
ice = "Ice", fire = "Fire", earth = "Earth", wind = "Wind";
...
if (choice == ice)

I would just do that:

if (choice == "Ice")

This works because std::string has the necessary overloads so it knows you want to do string comparison when you use == with a string literal like that. The same goes for std::string w, wU;, I'd just take them out and put the string literals where you print them.

Upvotes: 0

molbdnilo
molbdnilo

Reputation: 66371

You don't need to write classes in a bunch of files to clean that up.

You can come a long way if you get rid of some variable abundance, use descriptive names, and utilise the standard library.

One suggestion:

int main()
{
    const std::map<std::string, std::vector<std::string>> library =
    {
        {"Ice" , { "The Ice Gauntlet", "The Formal Ice Queen", "Frozen in Time", "Frost Lake" }},
        {"Fire", { "The Fire Gauntlet", "The Formal Fire Queen", "Hot Air", "Fire Lake" }},
        {"Earth", { "The Earth Gauntlet", "The Formal Earth Queen", "Stuck in Time", "The Swamp" }},
        {"Wind", { "The Wind Gauntlet", "The Formal Wind Queen", "Blown in Time", "Wind Lake" }}
    };

    std::string name;
    std::string lastName;
    std::cout << "Please enter your name" << std::endl;
    std::cin >> name;
    std::cout << "Please enter your lastname" << std::endl;
    std::cin >> lastName;
    std::string userAccount = name + lastName;

    std::cout << "Welcome, " << userAccount << std::endl;
    std::cout << "What kind of genre do you like to read? \n" << std::endl;
    for (const auto& entry: library)
    {
        std::cout << entry.first << "\n";
    }

    std::cout << "Please pick your genre\n" << std::endl;
    std::string choice;
    std::cin >> choice;
    auto it = library.find(choice);
    if (it != library.end())
    {
        std::cout << "Type yess if this is the genre of your choice\n";
        std::string answer;
        std::cin >> answer;
        for (const auto& title: it->second)
        {
            std::cout << title << "\n";
        }
    }
    else
    {
        std::cout << choice << " is not a known genre.";
    }
}

Upvotes: 2

Related Questions