HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

A simple Mastermind game in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mastermindsimplegame

Problem

Any idea to write this code more smarter and shorter? I also expect your advice about general organisation of the code.


Object of the Game


In your version of Mastermind, the computer will be the codemaker and one player will be the codebreaker. The computer picks a sequence of 4 pegs, each one being one of any of size colors.


The object of the game is to guess the exact positions of the colors in the computer's sequence in as few guesses as possible. After each guess, the computer gives you a score of exact and partial matches.


Rules



  • The sequence can contain pegs of colors: red, yellow, green, blue, white, black.



  • A color can be used any number of times in the sequence.



  • All four pegs of the secret sequence will contain a color - no blanks/empties are allowed.



  • Each guess must consist of 4 peg colors - no blanks.



  • The player has 12 guesses to find the secret sequence.





Scoring


For each of the pegs in your guess that is the correct color and in
the correct position, the computer will give you one small black peg to
the right of that move. If you score 4 small black pegs on a guess,
you have guessed the secret sequence.


For each of the pegs in your guess that is a correct color in an incorrect position, the computer will give you one small white peg to the right of that move. Together, there will be no more than four small black and white pegs for each move.


If none of the pegs in your guess is of a correct color, you will see no small pegs to the right of that move.


Sample scoring:





Requirements:



  • The players should be able to enter four colours as their guess. When they enter their guess, then your program should display their guess and next to the guess it should display the score. Make sure that you clearly visualise the guess and also the score next to it. You do not need to use graphics as long as you display the colours properly that will be enough.



-

Solution

The Good

The choice of variable and function names is very descriptive.
The code is properly indented. The consistent use of camelCase is excellent.

MAGIC NUMBERS

The term Magic Numbers refers to numerical constants in the code. A good programming practice is to
used named constants rather than numbers. Named constants make the code more self documenting, and
allow easier modification of the code. When a named constant is used, the code only needs to be
changed in one location rather than multiple locations. An example of this in the code would be to
increase or decrease the size of the arrays.

Example Named Constants:

#define ARRAY_SIZE    4
#define STRING_SIZE   10
#define RED           1
#define YELLOW        2
#define GREEN         3
#define BLUE          4
#define BLACK         5
#define WHITE         6
#define MAX_GUESS     12

void makeCode(char secretCode[ARRAY_SIZE][STRING_SIZE])
{
    int i, randColor;
    for(i=0; i<ARRAY_SIZE; i++)
    {
        randColor = 1 + rand() % 6;     //creates a number
        switch(randColor)       //converts number created to a string
        {
            case RED:
                strcpy(secretCode[i], "red");
                break;
            case YELLOW:
                strcpy(secretCode[i], "yellow");       
                break;
            case GREEN:
                strcpy(secretCode[i], "green");       
                break;
            case BLUE:
                strcpy(secretCode[i], "blue");       
                break;
            case BLACK:
                strcpy(secretCode[i], "white");       
                break;
            case WHITE:
                strcpy(secretCode[i], "black");       
                break;
        }
    }
}

void displayGuess(char guessCode[ARRAY_SIZE][STRING_SIZE], int blackPeg, int whitePeg)
{
    int i;
    printf("\nYour Guess\t\t\t\tYour Score\n");
    for(i=0; i < ARRAY_SIZE; i++)
        printf("%s ", guessCode[i]);
    printf("\t\t");
    for(i=0; i<blackPeg; i++)
        printf("black ");
    for(i=0; i<whitePeg; i++)
        printf("white ");
    printf("\n\n");
}


Note: The code above would be more efficient if you had an array of color strings and indexed into the
the array using randColor as follows:

void makeCode(char secretCode[ARRAY_SIZE][ARRAY_SIZE])
{
    int i, randColor;
    char *secretCodeColors[] =
        {
            "red",  // Multiple lines used to make it easier to add colors.
            "yellow",
            "green",
            "blue",
            "black",
            "white",
        };

    for(i=0; i<4; i++)
    {
        randColor = rand() % 6;
        strcpy(secretCode[i], secretCodeColors[randColor]);
    }
}


Not only does it make the function shorter, but indexing into the array is faster than the switch statement.

@MegaTom is correct; this code would be better using enums or named constants. The string compares are much
less efficient that integer compares.

Multiple Statements on a Line

To make future modifications easier, there should never be multiple statements on a single line. Let's say
some code needed to be added to each case in the switch statement. Each of the cases in the switch statement
would then be need to be broken into multiple lines which makes the edit more complex, and can create typos
during the edit. It is much easier to just add another statement by adding a single line, rather than
trying to add it to a single line.

case 1: strcpy(secretCode[i], "red");       break;
            case 1:
                strcpy(secretCode[i], "red");       
                break;


Generally it is better to assume that code will need to edited at some time in the future to add features or
fix bugs.

Prefer return over exit()

In the case of this program, since there is no error checking, there is no reason to use either exit() or return.
If there is error handling in the program, the exit() function should only be used if the program encounters an
error it can't correct deep in multiple function calls. The exit() function should only be used in standalone
programs, never in operating systems code. When using the exit() function, use the macros EXIT_SUCCESS and
EXIT_FAILURE that are defined in `.

Don't Repeat Yourself

When code is repeating itself, it is better to write another function rather than repeating the code. Then the
code only needs to be written and debugged once rather than multiple times. This is known as the DRY principle in software engineering.

Example:

The following code has loops that repeat:

``
void displayGuess(char guessCode[ARRAY_SIZE][STRING_SIZE], int blackPeg, int whitePeg)
{
int i;
printf("\nYour Guess\t\t\t\tYour Score\n");
for(i=0; i < ARRAY_SIZE; i++)
printf("%s ", guessCode[i]);
printf("\t\t");
for(i=0; i < blackPeg; i++)
printf("black ");
for(i=0; i < whitePeg; i++)
printf("white ");
printf("\n\n");
}

void showPeg(int pegCount, c

Code Snippets

#define ARRAY_SIZE    4
#define STRING_SIZE   10
#define RED           1
#define YELLOW        2
#define GREEN         3
#define BLUE          4
#define BLACK         5
#define WHITE         6
#define MAX_GUESS     12


void makeCode(char secretCode[ARRAY_SIZE][STRING_SIZE])
{
    int i, randColor;
    for(i=0; i<ARRAY_SIZE; i++)
    {
        randColor = 1 + rand() % 6;     //creates a number
        switch(randColor)       //converts number created to a string
        {
            case RED:
                strcpy(secretCode[i], "red");
                break;
            case YELLOW:
                strcpy(secretCode[i], "yellow");       
                break;
            case GREEN:
                strcpy(secretCode[i], "green");       
                break;
            case BLUE:
                strcpy(secretCode[i], "blue");       
                break;
            case BLACK:
                strcpy(secretCode[i], "white");       
                break;
            case WHITE:
                strcpy(secretCode[i], "black");       
                break;
        }
    }
}

void displayGuess(char guessCode[ARRAY_SIZE][STRING_SIZE], int blackPeg, int whitePeg)
{
    int i;
    printf("\nYour Guess\t\t\t\tYour Score\n");
    for(i=0; i < ARRAY_SIZE; i++)
        printf("%s ", guessCode[i]);
    printf("\t\t");
    for(i=0; i<blackPeg; i++)
        printf("black ");
    for(i=0; i<whitePeg; i++)
        printf("white ");
    printf("\n\n");
}
void makeCode(char secretCode[ARRAY_SIZE][ARRAY_SIZE])
{
    int i, randColor;
    char *secretCodeColors[] =
        {
            "red",  // Multiple lines used to make it easier to add colors.
            "yellow",
            "green",
            "blue",
            "black",
            "white",
        };

    for(i=0; i<4; i++)
    {
        randColor = rand() % 6;
        strcpy(secretCode[i], secretCodeColors[randColor]);
    }
}
case 1: strcpy(secretCode[i], "red");       break;
            case 1:
                strcpy(secretCode[i], "red");       
                break;
void displayGuess(char guessCode[ARRAY_SIZE][STRING_SIZE], int blackPeg, int whitePeg)
{
    int i;
    printf("\nYour Guess\t\t\t\tYour Score\n");
    for(i=0; i < ARRAY_SIZE; i++)
        printf("%s ", guessCode[i]);
    printf("\t\t");
    for(i=0; i < blackPeg; i++)
        printf("black ");
    for(i=0; i < whitePeg; i++)
        printf("white ");
    printf("\n\n");
}

void showPeg(int pegCount, char *colorString)
{
    for (i = 0; i < pegCount; i++)
        printf(colorString);
}

void displayGuess(char guessCode[ARRAY_SIZE][STRING_SIZE], int blackPeg, int whitePeg)
{
    int i;
    printf("\nYour Guess\t\t\t\tYour Score\n");
    for(i=0; i < ARRAY_SIZE; i++)
        printf("%s ", guessCode[i]);
    printf("\t\t");
    showPeg(blackPeg, "black ");
    showPeg(whitePeg, "white ");
    printf("\n\n");
}

Context

StackExchange Code Review Q#138339, answer score: 6

Revisions (0)

No revisions yet.