5
\$\begingroup\$

I'm trying to make a dice game with three dice and with rules.

  1. It costs 10 credits to roll the dice.
  2. If you get 6 6 6 you win 90 credits.
  3. If you get 5 5 5, 4 4 4, 3 3 3, 2 2 2, 1 1 1, you win 40 credits.
  4. If two of the dice have the same number you win 0 credits (because it costs 10 to roll).

Here's what I've done so far:

#include <iostream>
#include <stdlib.h>
#include <time.h>

int main(void) {

    srand((int)time(NULL));

    int diceRoll1, diceRoll2, diceRoll3;
    char yn;

    do {
        printf("\nDo you want to play? (y/n): ");
        scanf_s("%c", &yn);
        getchar();

        if (yn == 'n') {
            printf("Coward!");
            getchar();
            return 0;
        }

        diceRoll1 = (rand() % 6) + 1;
        diceRoll2 = (rand() % 6) + 1;
        diceRoll3 = (rand() % 6) + 1;

        printf("Your dices are %d %d %d \n", diceRoll1, diceRoll2, diceRoll3);

        if (diceRoll1 == 6 && diceRoll2 == 6 && diceRoll3 == 6) {
            printf("Congratulations! You won 90 credits.\n");
        }

        if (diceRoll1 == 5 && diceRoll2 == 5 && diceRoll3 == 5) {
            printf("Congratulations! You won 40 credits.\n");
        }

        if (diceRoll1 == 4 && diceRoll2 == 4 && diceRoll3 == 4) {
            printf("Congratulations! You won 40 credits.\n");
        }

        if (diceRoll1 == 3 && diceRoll2 == 3 && diceRoll3 == 3) {
            printf("Congratulations! You won 40 credits.\n");
        }

        if (diceRoll1 == 2 && diceRoll2 == 2 && diceRoll3 == 2) {
            printf("Congratulations! You won 40 credits.\n");
        }

        if (diceRoll1 == 1 && diceRoll2 == 1 && diceRoll3 == 1) {
            printf("Congratulations! You won 40 credits.\n");
        }

    } while (yn != 'n');


    getchar();
    return 0;

}

I have not implemented all of the rules and the ones I have done can probably be done in a better way. I don't have much experience, so I wonder what the best way is to have all the rules in the game and so the code looks good.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ I'm a bit unsure about the exact wording of the task. The "you win 0" sounds to me as if I still had to pay the 10 credits to actually throw the dice. And what does "win 90" mean? When I start with 1000 and roll a 666, do I then have 1080 or 1090 credit? \$\endgroup\$ Nov 30, 2019 at 13:58
  • 2
    \$\begingroup\$ Clarifying your understanding of the rules and also specifying which ones you have implemented would greatly improve this question. \$\endgroup\$
    – Edward
    Nov 30, 2019 at 18:06

2 Answers 2

4
\$\begingroup\$

Reducing the amount of code in any program is good because that reduces the possible errors or bugs in the code. Providing general functions that can be reused is a good habit to get into.

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code multiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

As shown in another answer all the if statements of the form

        if (diceRoll1 == **x** && diceRoll2 == diceRoll1 && diceRoll3 == diceRoll1) {
            printf("Congratulations! You won **YY** credits.\n");
        }

Can be reduced to a simple function that can be reused.

This code can also be simplified, although it is not absolutely necessary

    diceRoll1 = (rand() % 6) + 1;
    diceRoll2 = (rand() % 6) + 1;
    diceRoll3 = (rand() % 6) + 1;

Can be

static constexpr int dieFaces = 6;      // Allows the number of faces of the dice to be changed in one place
                                        // if this is used in a board game such as D & D

int rollOneDie()
{
    return (rand() % dieFaces) + 1;
}

void rollTheDice(int &die1, int &die2, int &die3)
{
    die1 = rollOneDie();
    die2 = rollOneDie();
    die3 = rollOneDie();
}

Any time I see variables with the names var1, var2 and var3 I see the potential for using an array instead of separate variables. The entire program might be simplified if instead of 3 separate variables diceRoll1, diceRoll2 and diceRoll3 there was one array of dice. This would also allow the program to be easily changed so that more or less dice are used.

static constexpr int diceCount = 3;
static constexpr int highScore = 90;
static constexpr int normalScore = 40;
static constexpr int lowScore = 10;
static constexpr int noScore = 0;

void rollTheDice2(int dice[])
{
    for (int i = 0; i < diceCount; i++)
    {
        dice[i] = rollOneDie();
    }
}

bool allTheDiceEqualThisNumber(int dice[],int faceValue)
{
    for (int i = 0; i < diceCount; i++)
    {
        if (dice[i] != faceValue)
        {
            return false;
        }
    }
    return true;
}

int earnCredit(int dice[])
{
    if (allTheDiceEqualThisNumber(dice, dieFaces))
    {
        return highScore;
    }
    else if (allTheDiceEqualThisNumber(dice, 1))
    {
        return lowScore;
    }
    else
    {
        for (int i = 2; i < dieFaces - 1; i++) {
            if (allTheDiceEqualThisNumber(dice, i)) {
                return normalScore;
            }
        }
    }
    return noScore;
}

void showDice(int dice[])
{
    printf("Your dices are ");
    for (int i = 0; i < diceCount; i++)
    {
        printf("%d ", dice[i]);
    }
    printf("\n");
}

void playGame()
{
    int diceRolls[diceCount];

    rollTheDice2(diceRolls);

    showDice(diceRolls);

    int newCredits = earnCredit(diceRolls);
    if (newCredits > 0)
    {
        printf("Congratulations! You won 90 credits.\n");
    }
}

Complexity

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

int main(void)
{
    srand((int)time(NULL));

    char yn;

    do {
        printf("\nDo you want to play? (y/n): ");
        scanf_s("%c", &yn);
        getchar();

        if (yn == 'n') {
            printf("Coward!");
            getchar();
            return 0;
        }

        playGame();

    } while (yn != 'n');


    getchar();
    return 0;
}

Magic Numbers

There are Magic Numbers in the main() function (6, 90, 40, 10), it might be better to create symbolic constants for them to make the code more readble and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

The possible symbolic constants are show in the examples for DRY Code

C versus C++

This question was originally tagged with C rather than C++. The strict version of C can't compile #include <iostream>, it uses #include <stdio.h> instead. Also input in C++ generally prefers

std::cin >> variableName;

over scanf, scanf_s or getchar() and

std::cout << "Your score was " << variableName << "\n";

over printf().

C++ also has a newer random number generator with better distribution using #include <random>.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ "variableName << std::cin;" Is that C++42 new syntax? I can't compile it even with gcc HEAD 10.0.1 20200 and -std=gnu++2a. Did you mean std::cin >> variableName? :) \$\endgroup\$
    – L. F.
    Jan 20, 2020 at 11:48
  • \$\begingroup\$ Better? I think I corrected it. \$\endgroup\$
    – pacmaninbw
    Jan 20, 2020 at 12:50
  • \$\begingroup\$ Great. Now it compiles ;) \$\endgroup\$
    – L. F.
    Jan 20, 2020 at 12:51
3
\$\begingroup\$

You could condense the code using or-eing conditions that match the same credit gain:

    if (diceRoll1 == 6 && diceRoll2 == 6 && diceRoll3 == 6) {
        printf("Congratulations! You won 90 credits.\n");
    }

    if ((diceRoll1 == 5 && diceRoll2 == 5 && diceRoll3 == 5) || 
        (diceRoll1 == 4 && diceRoll2 == 4 && diceRoll3 == 4) || 
        (diceRoll1 == 3 && diceRoll2 == 3 && diceRoll3 == 3) ||
        (diceRoll1 == 2 && diceRoll2 == 2 && diceRoll3 == 2) ||
        (diceRoll1 == 1 && diceRoll2 == 1 && diceRoll3 == 1)) {
        printf("Congratulations! You won 40 credits.\n");
    }

I have not implemented all of the rules and the ones I have done can probably be done in a better way?

You could also make this code more flexible and extendible using function pointers and an array of these:

typedef int (*fn_gamerule(int,int,int));

int gamerule1(int,int,int);
int gamerule2(int,int,int);

// ...

fn_gamerule gamerules[] = { gamerule1, gamerule2, NULL };

// ...
int credits = 0;
for(fn_gamerule rule = gamerules; rule != NULL; ++rule) {
    credits = rule(diceRoll1m,diceRoll2,diceRoll3);
    if(credits  > 0) {
        printf("Congratulations! You won %d credits.\n",credits);
    }
}

// ...

int gamerule1(int dr1,int dr2,int dr3) {
    if(dr1 == 6 && dr2 == 6 && dr3 == 6) {
       return 90;
    }
    return 0;
}

int gamerule2(int dr1,int dr2,int dr3) {
    if ((dr1 == dr2 && dr2 == dr3) &&
        (dr1 >= 1 && dr1 < 6)) {
         return 40;
    }
    return 0;
}

Designing your code in that way will also help you to keep track of the credits in repeated iterations of the dice roll.

int rolldice_rule(int,int,int);
int allsame_rule(int,int,int);

// ...

fn_gamerule gamerules[] = { rolldice_rule, allsame_rule, NULL };

int total_credits = 0;
do {
    printf("\nDo you want to play? (y/n): ");
    scanf_s("%c", &yn);
    // getchar(); <= confusing user ecperience, not necessary

    if (yn == 'n') {
       printf("Coward!");
       // getchar(); <= confusing user ecperience, not necessary
       return 0;
    }

    diceRoll1 = (rand() % 6) + 1;
    diceRoll2 = (rand() % 6) + 1;
    diceRoll3 = (rand() % 6) + 1;

    printf("Your dices are %d %d %d \n", diceRoll1, diceRoll2, diceRoll3);

    int credits = 0;
    for(fn_gamerule rule = gamerules; rule != NULL; ++rule) {
        credits = rule(diceRoll1m,diceRoll2,diceRoll3);
        if(credits  > 0) {
            printf("Congratulations! You won %d credits.\n",credits);
        } 
        total_credits += credits;
    }
} while (yn != 'n');

// ...

int rolldice_rule(int dr1,int dr2,int dr3) {
    return -10;
}

int allsame_rule(int,int,int) {
    if (diceRoll1 == diceRoll2 == diceRoll3) {
        switch(diceRoll1) {
            case 6:
                return 90;
            default:
                return 40;
        }
    }
    return 0;
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.