Reputation: 29
I'm trying to simplify a switch statement so it doesn't take up so much room. I feel like I just have too much repeated code. I don't know if there is a better statement to use or a way to clean up this code. I'm also very new to C++ so any tips would help.
This is what I made for drawing a die
void Dice::DrawDie()
{
switch (LastRoll)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
switch (LastRoll2)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
switch (LastRoll3)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
switch (LastRoll4)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
switch (LastRoll5)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
}
Then I use a separate area for rolling the dice one for each die. I make a new die for each one.
void Dice::Dice1()
{
int RollNum;
LastRoll = 0;
TotalRolls++;
//Gens Rand Num 1-6
RollNum = (rand() % 6) + 1;
LastRoll = RollNum;
}
Upvotes: 1
Views: 227
Reputation: 1
You can store just the die faces in a std::map<int,std::array<const char*,5>>
and separate string literals for better readability:
const std::map<int,std::array<const char*,5>> die_faces = {
{ 1, { "+---+",
"| |",
"| o |",
"| |",
"+---+" } },
{ 2, { "+---+",
"|o |",
"| |",
"| o|",
"+---+" } },
{ 3, { "+---+",
"|o |",
"| o |",
"| o|",
"+---+" } },
{ 4, { "+---+",
"|o o|",
"| |",
"|o o|",
"+---+" } },
{ 5, { "+---+",
"|o o|",
"| o |",
"|o o|",
"+---+" } },
{ 6, { "+---+",
"|o o|",
"|o o|",
"|o o|",
"+---+" } }
};
And access these like
void output_dies(int roll1, int roll2) {
std::cout << "\n";
for(int i = 0; i < 5; ++i) {
std::cout << die_faces.at(roll1)[i] << "\t" << die_faces.at(roll2)[i] << "\n";
}
std::cout << "\n";
}
and
output_dies(LastRoll,LastRoll2);
No more switch needed at all.
Upvotes: 3
Reputation: 15446
You have a LOT of repeated code. A good rule to abide for clean code is DRY--Don't Repeat Yourself (after which my coworker will always say "What?" to make me repeat the description).
In this case, you can avoid all of the copy-paste by making a function, display_die()
like:
std::string str_rev(std::string str) {
std::reverse(str.begin(), str.end());
return str;
}
void display_die(int val) {
if(val < 1 || val > 6) { return; } // Bad value
std::vector<std::string> first_face = {" ", " o", " o", "o o", "o o", "o o"};
std::vector<std::string> second_face = {" o ", " ", " o ", " ", " o ", "o o"};
std::string lead = "\n ------- \n| |\n| ";
std::string split = " |\n| ";
std::string trail = " |\n| |\n ------- \t";
std::cout << lead << first_face[val - 1] << split
<< second_face[val - 1] << split
<< str_rev(first_face[val - 1]) << trail;
}
See it run here: https://ideone.com/lEJDzK
Upvotes: 1
Reputation: 122133
You have several blocks that are exactly the same apart from one variable FOO
switch (FOO)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
Use a function:
void output(int i) {
switch(i) {
case 1: //...
case 2: //...
}
}
Then your code becomes:
void Dice::DrawDie() {
output(LastRoll);
output(LastRoll2);
output(LastRoll3);
output(LastRoll4);
output(LastRoll5);
}
Next you probably want to store the rolls in a container, then the method is simply:
void Dice::DrawDie() {
for (auto& roll : last_rolls) output(roll);
}
Upvotes: 0
Reputation: 40842
Create a function void DrawDie(int RollNum)
, and call it with DrawDie(LastRoll1)
, …
void DrawDie(int RollNum) {
switch (RollNum)
{
case 1: cout << "\n ------- \n| |\n| |\n| o |\n| |\n| |\n ------- \t";
break;
case 2: cout << "\n ------- \n| |\n| o |\n| |\n| o |\n| |\n ------- \t";
break;
case 3: cout << "\n ------- \n| |\n| o |\n| o |\n| o |\n| |\n ------- \t";
break;
case 4: cout << "\n ------- \n| |\n| o o |\n| |\n| o o |\n| |\n ------- \t";
break;
case 5: cout << "\n ------- \n| |\n| o o |\n| o |\n| o o |\n| |\n ------- \t";
break;
case 6: cout << "\n ------- \n| |\n| o o |\n| o o |\n| o o |\n| |\n ------- \t";
}
}
And \n ------- \n| |\n|
and |\n| |\n ------- \t
could be moved before and after the switch.
void DrawDie(int RollNum) {
cout << "\n ------- \n| |\n|";
switch (RollNum)
{
case 1: cout << " |\n| o |\n| ";
break;
case 2: cout << " o |\n| |\n| o ";
break;
case 3: cout << " o |\n| o |\n| o ";
break;
case 4: cout << " o o |\n| |\n| o o ";
break;
case 5: cout << " o o |\n| o |\n| o o ";
break;
case 6: cout << " o o |\n| o o |\n| o o ";
}
cout << "|\n| |\n ------- \t";
}
Upvotes: 0