Gamez
Gamez

Reputation: 29

How can I simplify a multi switch case statement?

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

Answers (4)

πάντα ῥεῖ
πάντα ῥεῖ

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.

Live demo

Upvotes: 3

scohe001
scohe001

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

463035818_is_not_an_ai
463035818_is_not_an_ai

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

t.niese
t.niese

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

Related Questions