Reputation: 33
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
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
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