A simple Mastermind game in C

Question

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

1. The sequence can contain pegs of colors: red, yellow, green, blue, white, black.
2. A color can be used any number of times in the sequence.
3. All four pegs of the secret sequence will contain a color - no blanks/empties are allowed.
4. Each guess must consist of 4 peg colors - no blanks.
5. 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:

1. 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.

2. Different visualisation approach can be used, for example in your program when you do the computation you can assign an integer number to each colour, but then you can visualise it as follows which is based on Figure 1:

   Your guess:              | Your score:
   ---------------------------------------------------
   Blue Yellow Green Red    | Black Black White White
   ---------------------------------------------------
   Black Yellow White Red   | Black White
   ---------------------------------------------------
   Yellow Blue White Yellow | 
   

3. After the player completes playing the came once (after either they win or they had 12 guesses), your program should ask the user if they would like to continue if they do then your program should generate a new code.

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#include<time.h>

void makeCode(char secretCode[4][10])
{
    int i, randColor;
    for(i=0; i<4; i++)  
    {
        randColor = 1 + rand() % 6;     //creates a number 
        switch(randColor)       //converts number created to a string
        {
            case 1: strcpy(secretCode[i], "red");       break;
            case 2: strcpy(secretCode[i], "yellow");    break;
            case 3: strcpy(secretCode[i], "green");     break;
            case 4: strcpy(secretCode[i], "blue");      break;
            case 5: strcpy(secretCode[i], "white");     break;
            case 6: strcpy(secretCode[i], "black");     break;
        }
    }   
}

void guess(char guessCode[4][10])
{
    printf("\nEnter your guess:\n");
    for(int i=0; i<4; i++)
        scanf("%s", guessCode[i]);  
}

void codeCheck(char secretCode[4][10], char guessCode[4][10], int *blackPeg, int *whitePeg)
{
    int i, j, checkSecret[4] = {1,1,1,1}, checkGuess[4] = {1,1,1,1};
    *blackPeg = *whitePeg = 0;

    for(i=0; i<4; i++)      //if secret and guess's position and color are same, blackpeg increases and mark "check"
        if(strcmp(guessCode[i], secretCode[i]) == 0)   
        {
            ++*blackPeg;
            checkSecret[i] = checkGuess[i] = 0;
        }            

    for(i=0; i<4; i++)
        for(j=0; j<4; j++)       
            if(strcmp(secretCode[i],guessCode[j]) == 0  &&  checkGuess[i]  &&  checkSecret[j]  &&  i != j)        
            {// determines crushes and eliminates extra whitePegs 
                ++*whitePeg;
                checkSecret[j] = checkGuess[i] = 0;
            }
}

void displayGuess(char guessCode[4][10], int blackPeg, int whitePeg)
{   
    int i;
    printf("\nYour Guess\t\t\t\tYour Score\n");   
    for(i=0; i<4; 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");
}

int main()
{
    srand(time(NULL));
    int i, option=1, blackPeg, whitePeg, wrongGuess;
    char secretCode[4][10], guessCode[4][10];
    while(1)
    {
        printf("MASTER MIND! \nPress 1 to start game \nPress any number to exit\n\n");
        scanf("%d", &option);        
        if(option == 1)
        {
            makeCode(secretCode);
            for(wrongGuess=1; wrongGuess<=12; wrongGuess++)      //gives 12 rights to guess
            {
                guess(guessCode);
                codeCheck(secretCode, guessCode, &blackPeg, &whitePeg);
                displayGuess(guessCode, blackPeg, whitePeg);
                if(blackPeg == 4)           //if player guess correct all, than the game finishes
                {
                    printf("You Win!\n\n\n\n");  break;
                }
            }
        if(wrongGuess == 13)        //if player cannot guess correct colors in 12 rounds, he losts
            printf("\nYou Lost!\nSecret Code: %s %s %s %s\n\n\n\n\n", secretCode[0], secretCode[1], secretCode[2], secretCode[3]);  
        }
        else
            exit(1);
    }
}

Answer 1

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 <stdlib.h>.

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, 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");
}

Decrease Function Complexity

Another software engineering principle is the Single Responsibility Principle. A function should only be responsible for one action; this makes each function easier to read, write, debug and use multiple times. It is much better to write smaller more concise functions that can be used in multiple places and only need to be debugged once. The functions codeCheck() and main() would both benefit from applying this principle.

General Comments

The program should guide the user better. It's not clear what the input should be; there should be prompts for each input. The program might be more fun if the user could enter colors rather than numbers.

Answer 2

Good job on the code. This is better then I did the first time I tried this problem. But I do see three big problems:

1. Stores codes as strings

I think that it would be best to use a number/enum for the color, not a string. This will use less memory and simplify comparing later, as well as basically fixing problem 2.

2. Silently fails on invalid colors

As your code stands, a player may put in invalid colors and will not get a warning. So if i guess bleu purple gray orange, it will say I got nothing, and continue. I think it would be better to tell the user that they are guessing invalid colors. (This is made even worse by the fact that the user is not told what colors are good in the first place.) If you validate as they enter the colors that they entering valid colors, that can fix the problem.

3. Case sensitive

Red RED Green Black will fail the comparison the same way as above, because you use strcmp, not strcmpi.

Answer 3

You should replace your #defines with an enum, defines just copy and paste stuff, enums are actually part of the language.

Also, your spacing makes it a bit more difficult to read, but I've seen much worse.