Minesweeper C++

Question

This is my improvement of a minesweeper game I took from the internet, which I am quite proud of. Nevertheless I am willing to hear your opinions, suggestions, and comments.

/* Minesweeper V2.0
Written by:-Shivam Shekhar
Improved and understood by Eshel BM*/
#include<windows.h>
#include<stdio.h>
#include<conio.h>
#include<time.h>
//read only variables - can be changed.
#define NUM_OF_BOMBS 60 //HAS TO BE LESS THAN 100 AND LESS THAN HEIGHT * WIDTH
#define HEIGHT 13 //the amount of actual "buttons" in the height of the grid
#define WIDTH 25 //the amount of actual "buttons" in the width of the grid
#define OFFSET 3 //how far away is the minefield from the sides of the screen
#define OFFSET_FLAG_W 20 //how far away is the flags counter from the left of the screen
#define OFFSET_FLAG_H 1 //how far away is the flags counter from the top of the screen
#define BUTTON_CH 219 //this is the ascii value of a blank square - character: '█'
#define BOMB_CH 15 //'☼'
//No change allowed
#define SIZE SIDE_H * SIDE_W
#define SIDE_W (WIDTH * 2 - 1) //the horizontal side of the grid of the minefield (buttons + space between buttons)
#define SIDE_H (HEIGHT * 2 - 1) //the vertical side of the grid of the minefield (buttons + space between buttons)
#define FLAGS_DISP_SIZE 4
//colors (b= background, f=foregruond):
#define GRAY_F FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN
#define WHITE_F GRAY_F | FOREGROUND_INTENSITY
#define GRAY_B BACKGROUND_BLUE | BACKGROUND_RED | BACKGROUND_GREEN
#define WHITE_B GRAY_B | BACKGROUND_INTENSITY
//function declaration:
int menu(HANDLE out, HANDLE in, bool print);
void instructions();
void updateFlagsDisp(CHAR_INFO * flags, int flagsLeft, bool initColor);
void initFields(CHAR_INFO * mines, CHAR_INFO * map);
bool validClick(COORD clickLocation, CHAR_INFO * field);
void dropMines(CHAR_INFO * map, COORD firstClick);
void fillMap(CHAR_INFO * map);
void reveal(COORD clickLocation, CHAR_INFO * map, CHAR_INFO * field);
void color(CHAR_INFO * field, COORD location);
void removeFlag(CHAR_INFO * field, COORD location, int * numOfFlags);
void placeFlag(CHAR_INFO * field, COORD location, int * numOfFlags);
bool checkOver(CHAR_INFO * map, CHAR_INFO * minefield, COORD pos);
void showMistakes(CHAR_INFO * map, CHAR_INFO * minefield, COORD pos);
int main()
{
 DWORD info;
 HANDLE in = GetStdHandle(STD_INPUT_HANDLE);
 HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);
 INPUT_RECORD input;
 CHAR_INFO minefield[SIZE], //user interaction display
 map[SIZE], //solved area display
 flags[FLAGS_DISP_SIZE]; //Flags display
 SMALL_RECT fieldsRect = { OFFSET, OFFSET, OFFSET + SIDE_W, OFFSET + SIDE_H }; //rectangle for fields (minefield & map)
 SMALL_RECT flagsRect = { OFFSET_FLAG_W, OFFSET_FLAG_H, OFFSET_FLAG_W + FLAGS_DISP_SIZE, OFFSET_FLAG_H }; //rectangle for flags display
 COORD fieldsGrid = { SIDE_W, SIDE_H };
 COORD flagsGrid = { FLAGS_DISP_SIZE, 1 };
 COORD origin = { 0, 0 };
 COORD pos; //track where user clicked
 int mines = 0, flagsLeft;
 //FLAGS:
 bool begin = false, flagsPutting = false, changed = true, finished = false, print = true;
 system("color 3F");
 do
 {
 //start screen:
 switch (menu(out, in, print))
 {
 case 0:
 print = false;
 break;
 case 1:
 begin = true;
 break;
 case 2:
 print = true;
 instructions();
 break;
 }
 } while (!begin);
 int width = OFFSET_FLAG_W + FLAGS_DISP_SIZE + 1 > SIDE_W + OFFSET * 2 ? OFFSET_FLAG_W + FLAGS_DISP_SIZE + 1 : SIDE_W + OFFSET * 2;
 System::Console::SetWindowSize(width, SIDE_H + OFFSET * 2);
start:
 system("cls");
 FlushConsoleInputBuffer(in);
 SetConsoleMode(in, ENABLE_MOUSE_INPUT);
 flagsLeft = NUM_OF_BOMBS;
 //Initialize flags display
 updateFlagsDisp(flags, flagsLeft, true);
 WriteConsoleOutput(out, flags, flagsGrid, origin, &flagsRect);
 //Initialize minefield and map display
 initFields(minefield, map);
 WriteConsoleOutput(out, minefield, fieldsGrid, origin, &fieldsRect);
 //taking user input for 1st square
 while (true) //wait until user clicks a valid button
 {
 ReadConsoleInput(in, &input, 1, &info);
 if (input.EventType == MOUSE_EVENT)
 {
 pos = input.Event.MouseEvent.dwMousePosition;
 pos.X -= OFFSET;
 pos.Y -= OFFSET;
 if (input.Event.MouseEvent.dwButtonState == FROM_LEFT_1ST_BUTTON_PRESSED && validClick(pos, minefield))
 break;
 }
 }
 //random placing of mines
 dropMines(map, pos);
 //filling rest of map based on mines placed
 fillMap(map);
 //To see map, uncomment next 5 lines:
 /*fieldsRect.Left += SIDE_W + OFFSET;
 fieldsRect.Right += SIDE_W + OFFSET;
 WriteConsoleOutput(out,map,fieldsGrid,origin,&fieldsRect);
 fieldsRect.Left -= SIDE_W + OFFSET;
 fieldsRect.Right -= SIDE_W + OFFSET;*/
 reveal(pos, map, minefield);
 WriteConsoleOutput(out, minefield, fieldsGrid, origin, &fieldsRect);
 //finally the game begins
 finished = false;
 while (!finished)
 {
 ReadConsoleInput(in, &input, 1, &info);
 if (input.EventType != MOUSE_EVENT)
 continue;
 if (input.Event.MouseEvent.dwEventFlags != 0)
 continue;
 pos = input.Event.MouseEvent.dwMousePosition;
 pos.X -= OFFSET;
 pos.Y -= OFFSET;
 if (input.Event.MouseEvent.dwButtonState == FROM_LEFT_1ST_BUTTON_PRESSED)
 {
 if (validClick(pos, minefield))
 {
 //user clicked a square in minefield
 if (minefield[pos.X + pos.Y * SIDE_W].Char.AsciiChar == 'F')
 removeFlag(minefield, pos, &flagsLeft);
 else if (!flagsPutting)
 reveal(pos, map, minefield);
 else if (flagsLeft > 0)
 placeFlag(minefield, pos, &flagsLeft);
 else
 changed = false;
 }
 else if (pos.X == OFFSET_FLAG_W - OFFSET && pos.Y == OFFSET_FLAG_H - OFFSET)
 {
 //user clicked flags button
 flagsPutting = !flagsPutting;
 if (flagsPutting)
 flags[0].Attributes = BACKGROUND_GREEN | BACKGROUND_INTENSITY | WHITE_F;
 else
 flags[0].Attributes = FOREGROUND_BLUE | FOREGROUND_INTENSITY | WHITE_B;
 }
 else
 changed = false;
 }
 else if (input.Event.MouseEvent.dwButtonState == RIGHTMOST_BUTTON_PRESSED)
 {
 if (minefield[pos.X + pos.Y * SIDE_W].Char.AsciiChar != 'F' && flagsLeft > 0 && validClick(pos, minefield))
 placeFlag(minefield, pos, &flagsLeft);
 else
 changed = false;
 }
 else
 changed = false;
 if (changed)
 {
 WriteConsoleOutput(out, minefield, fieldsGrid, origin, &fieldsRect);
 updateFlagsDisp(flags, flagsLeft, false);
 WriteConsoleOutput(out, flags, flagsGrid, origin, &flagsRect);
 }
 else
 changed = true;
 //check if game is over
 finished = checkOver(map, minefield, pos);
 if (finished)
 {
 showMistakes(map, minefield, pos);
 WriteConsoleOutput(out, minefield, fieldsGrid, origin, &fieldsRect);
 printf("\nPlay again?(y / n)");
 switch (_getch())
 {
 case 'y':
 case 'Y':
 goto start;
 break;
 case 'n':
 case 'N':
 break;
 default:
 printf(" Invalid input, exiting game");
 _getch();
 break;
 }
 }
 }
 return 0;
}
//prints the menu in the middle of the screen, waits for user input and returns it. (0=invalid input, 1=play, 2=rules);
int menu(HANDLE out, HANDLE in, bool print)
{
 const int INVALID = 0, PLAY = 1, RULES = 2;
 DWORD info;
 INPUT_RECORD input;
 COORD positionTitle = { 30, 5 }; //where label is located
 COORD positionPlay = { 30, 10 };
 COORD positionRules = { 30, 11 };
 COORD click; //where user clicked
 if (print)
 {
 system("cls");
 SetConsoleCursorPosition(out, positionTitle); //set print start position on the screen
 printf("Minesweeper");
 SetConsoleCursorPosition(out, positionPlay);
 printf("1. Play ");
 SetConsoleCursorPosition(out, positionRules);
 printf("2. Rules");
 }
 SetConsoleMode(in, ENABLE_MOUSE_INPUT);
 ReadConsoleInput(in, &input, 1, &info);
 if (input.EventType == MOUSE_EVENT)
 {
 if (input.Event.MouseEvent.dwButtonState != FROM_LEFT_1ST_BUTTON_PRESSED)
 return INVALID;
 click = input.Event.MouseEvent.dwMousePosition;
 if (positionPlay.Y == click.Y && click.X >= positionPlay.X && click.X <= positionPlay.X + 6)
 return PLAY;
 if (positionRules.Y == click.Y && click.X >= positionRules.X && click.X <= positionRules.X + 7)
 return RULES;
 else
 return INVALID;
 }
 if (input.EventType == KEY_EVENT)
 {
 char ch = input.Event.KeyEvent.uChar.AsciiChar;
 if (ch == '1')
 return PLAY;
 else if (ch == '2')
 return RULES;
 else
 return INVALID;
 }
 return INVALID;
}
//prints the instructions and waits for user input
void instructions()
{
 system("cls");
 printf("Minesweeper v2.0\n");
 printf("1.Your aim is to successfully flag all the mines without opening any mines\n");
 printf("2.Use your mouse to open the squares. Left click on a square to open it\n");
 printf("3.To flag a square, right click on it, or press the 'F' which is located\nover the minefield\n");
 printf("4.To remove a flag from the square, simply left click on the flagged square.\n");
 printf("5.If you open a mine, you lose\n");
 printf("6.If you open a square with a number written on it, the number shows\nhow many mines are there in the adjacent 8 squares\n");
 printf("\nFor eg:\n%c %c %c\n\n%c 4 %c\n\n%c %c %c", BUTTON_CH, BUTTON_CH, BUTTON_CH, BUTTON_CH, BUTTON_CH, BUTTON_CH, BUTTON_CH, BUTTON_CH);
 printf("\nHere '4' means that there are 4 mines in the remaining uncovered squares");
 printf("\n\n\n");
 system("pause");
}
//initiates the flags display with the current number of flags left
void updateFlagsDisp(CHAR_INFO * flags, int flagsLeft, bool initColor)
{
 flags[0].Char.AsciiChar = 'F';
 flags[1].Char.AsciiChar = ':';
 flags[2].Char.AsciiChar = flagsLeft > 9 ? '0' + (int)flagsLeft / 10 : ' ';
 flags[3].Char.AsciiChar = flagsLeft % 10 + '0';
 if (!initColor)
 return;
 for (int i = 0; i < FLAGS_DISP_SIZE; i++)
 flags[i].Attributes = FOREGROUND_BLUE | FOREGROUND_INTENSITY | WHITE_B;
}
//makes all button in fields unopened, and spaces between buttons match background
void initFields(CHAR_INFO * minefield, CHAR_INFO * map)
{
 int i, j;
 for (j = 0; j < SIDE_H; j++)
 {
 if (j % 2 == 0)
 {
 for (i = 0; i < SIDE_W; i++)
 {
 if (i % 2 == 0)
 {
 //init button:
 minefield[i + j * SIDE_W].Char.AsciiChar = BUTTON_CH;
 minefield[i + j * SIDE_W].Attributes = 0;//GRAY_F;
 map[i + j * SIDE_W].Char.AsciiChar = BUTTON_CH;
 }
 else
 {
 //init space between buttons
 minefield[i + j * SIDE_W].Char.AsciiChar = ' ';
 minefield[i + j * SIDE_W].Attributes = BACKGROUND_BLUE | BACKGROUND_GREEN; //Matches background color
 map[i + j * SIDE_W].Char.AsciiChar = ' ';
 map[i + j * SIDE_W].Attributes = 01;
 }
 }
 }
 else
 {
 //init space between buttons
 for (i = 0; i < SIDE_W; i++)
 {
 minefield[i + j * SIDE_W].Char.AsciiChar = ' ';
 minefield[i + j * SIDE_W].Attributes = BACKGROUND_BLUE | BACKGROUND_GREEN;
 map[i + j * SIDE_W].Char.AsciiChar = ' ';
 map[i + j * SIDE_W].Attributes = 01;
 }
 }
 }
}
//checks if there is something to respond to the user's click
bool validClick(COORD clickLocation, CHAR_INFO * field)
{
 //check if click is within rectangle:
 if (clickLocation.X < 0 || clickLocation.X > SIDE_W)
 return false;
 if (clickLocation.Y < 0 || clickLocation.Y > SIDE_H)
 return false;
 //check if click is in a button:
 if ((clickLocation.X) % 2 != 0 || clickLocation.Y % 2 != 0)
 return false;
 //check if the button clicked was not clicked previously:
 unsigned char ch = field[clickLocation.X + SIDE_W * clickLocation.Y].Char.AsciiChar;
 if (ch != BUTTON_CH && ch != 'F')
 return false;
 return true;
}
//fills the map with NUM_OF_BOMBS mines in random places, and not where the user first clicked
void dropMines(CHAR_INFO * map, COORD firstClick)
{
 int mines = 0, i, j;
 srand(time(NULL));
 while (mines != NUM_OF_BOMBS)
 {
 i = 2 * (rand() % WIDTH);
 j = 2 * SIDE_W * (rand() % HEIGHT);
 if (map[i + j].Char.AsciiChar != BOMB_CH && (i + j) != (firstClick.X + firstClick.Y * SIDE_W))
 {
 map[i + j].Char.AsciiChar = BOMB_CH;
 map[i + j].Attributes = FOREGROUND_RED | FOREGROUND_INTENSITY;
 mines++;
 }
 }
}
//fills the map that contains mines with numbers that represent the number of mines in the surrounding squares
void fillMap(CHAR_INFO * map)
{
 int i, j, k, l, minesSurrounding = 0;
 for (j = 0; j < HEIGHT; j++)
 {
 for (i = 0; i < WIDTH; i++)
 {
 if (map[2 * i + 2 * j * SIDE_W].Char.AsciiChar != BOMB_CH)
 {
 //count bombs on the surrounding squares
 for (k = j - 1; k <= j + 1; k++)
 {
 for (l = i - 1; l <= i + 1; l++)
 {
 if ((unsigned char)map[l * 2 + k * 2 * SIDE_W].Char.AsciiChar == BOMB_CH && l >= 0 && 0 <= k && l < WIDTH && k < HEIGHT)
 {
 minesSurrounding++;
 }
 }
 }
 map[i * 2 + j * 2 * SIDE_W].Char.AsciiChar = minesSurrounding + '0';
 map[i * 2 + j * 2 * SIDE_W].Attributes = FOREGROUND_BLUE | FOREGROUND_INTENSITY | WHITE_B;
 minesSurrounding = 0;
 }
 }
 }
}
//shows the content of a location in the miefield, if the content is '0', shows all its surroundings
void reveal(COORD clickWhere, CHAR_INFO * map, CHAR_INFO * minefield)
{
 int x = clickWhere.X, y = clickWhere.Y;
 if (minefield[x + y * SIDE_W].Char.AsciiChar == 'F')
 return; //does not reveal what is behind a flag
 minefield[x + y * SIDE_W].Char.AsciiChar = map[x + y * SIDE_W].Char.AsciiChar;
 map[x + y * SIDE_W].Char.AsciiChar = BUTTON_CH; //to know that this location was already discovered
 if ((unsigned char)minefield[x + y * SIDE_W].Char.AsciiChar == '0')
 {
 for (int k = y - 2; k <= y + 2; k += 2)
 {
 for (int l = x - 2; l <= x + 2; l += 2)
 {
 if (0 <= l && 0 <= k && l < SIDE_W && k < SIDE_H && (unsigned char)map[l + k * SIDE_W].Char.AsciiChar != BUTTON_CH)
 {
 COORD newClick = { l, k };
 reveal(newClick, map, minefield); //recursive call to show all squares surrounding a '0', since none of them contains a bomb.
 }
 }
 }
 }
 color(minefield, clickWhere);
}
//colors the character based on the number
void color(CHAR_INFO * field, COORD pos)
{
 field[pos.X + pos.Y * SIDE_W].Attributes = WHITE_B;
 switch (field[pos.X + pos.Y * SIDE_W].Char.AsciiChar)
 {
 case '0':
 field[pos.X + pos.Y * SIDE_W].Char.AsciiChar = ' ';
 break;
 case '1':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_BLUE | FOREGROUND_INTENSITY;
 break;
 case '2':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_GREEN | FOREGROUND_INTENSITY;
 break;
 case '3':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_RED | FOREGROUND_INTENSITY;
 break;
 case '4':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_BLUE;
 break;
 case '5':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_RED;
 break;
 case '6':
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_GREEN | FOREGROUND_BLUE;
 break;
 case '7':
 field[pos.X + pos.Y * SIDE_W].Attributes |= 0;
 break;
 case '8':
 field[pos.X + pos.Y * SIDE_W].Attributes |= GRAY_F;
 break;
 case BOMB_CH:
 field[pos.X + pos.Y * SIDE_W].Attributes |= FOREGROUND_RED | FOREGROUND_INTENSITY;
 }
}
//removes a flag from the minefield and updates the counter
void removeFlag(CHAR_INFO * field, COORD location, int * numOfFlags)
{
 field[location.X + location.Y * SIDE_W].Char.AsciiChar = BUTTON_CH;
 field[location.X + location.Y * SIDE_W].Attributes = 0;
 (*numOfFlags)++;
}
//places a flag on the minefield and updates the counter
void placeFlag(CHAR_INFO * field, COORD location, int * numOfFlags)
{
 field[location.X + location.Y * SIDE_W].Char.AsciiChar = 'F';
 field[location.X + location.Y * SIDE_W].Attributes = BACKGROUND_RED | BACKGROUND_BLUE | BACKGROUND_INTENSITY;
 (*numOfFlags)--;
}
//checks if the game is over
//two possible situations:
//1) user clicked a bomb
//2) user has won
bool checkOver(CHAR_INFO * map, CHAR_INFO * minefield, COORD pos)
{
 int correctFlags = 0, y, x;
 if (minefield[pos.X + pos.Y * SIDE_W].Char.AsciiChar == BOMB_CH)
 {
 printf("game over");
 return true;
 }
 for (y = 0; y < HEIGHT; y++)
 {
 for (x = 0; x < WIDTH; x++)
 {
 if (minefield[x * 2 + y * SIDE_W * 2].Char.AsciiChar == 'F')
 {
 if (map[x * 2 + y * SIDE_W * 2].Char.AsciiChar == BOMB_CH)
 correctFlags++;
 else
 break;
 }
 }
 }
 if (correctFlags == NUM_OF_BOMBS)
 {
 printf("You win!");
 return true;
 }
 return false;
}
//At the end of the game, marks the places where the user has put a flag which is not over a bomb
void showMistakes(CHAR_INFO * map, CHAR_INFO * minefield, COORD pos)
{
 const int MISTAKE_CHAR = 'X';
 int y, x;
 for (y = 0; y < HEIGHT; y++)
 {
 for (x = 0; x < WIDTH; x++)
 {
 if (minefield[x * 2 + y * SIDE_W * 2].Char.AsciiChar == 'F' && map[x * 2 + y * SIDE_W * 2].Char.AsciiChar != BOMB_CH)
 {
 minefield[x * 2 + y * SIDE_W * 2].Char.AsciiChar = 'X';
 minefield[x * 2 + y * SIDE_W * 2].Attributes = 0 | FOREGROUND_RED | FOREGROUND_INTENSITY;
 }
 }
 }
}

• syb0rg: This looks like C, not C++. They are different languages, and tagging which one you developed for is important as you will get different reviews as such.
• forsvarir: @syb0rg I think it's on the C style side of c++. I've never seen anything like System::Console::SetWindowSize(width, SIDE_H + OFFSET * 2) in C. It's calling out to the .NET clr...
• syb0rg: @forsvarir I think you found the only line of C++ in the code, which I admittedly missed. Good catch!

Answer 1

C++ Vs C

As has been said in the comments, your code reads very much like it's been written in C. You're using C functions like printf instead of the more C++ cout and you haven't broken your logic down into classes etc.

Commented out Code

Commented out code creates noise that distracts from the rest of the code. If you want to have conditional compilation, it's better to build it in to your process. So, instead of:

//To see map, uncomment next 5 lines:
/*fieldsRect.Left += SIDE_W + OFFSET;
fieldsRect.Right += SIDE_W + OFFSET;
WriteConsoleOutput(out,map,fieldsGrid,origin,&fieldsRect);
fieldsRect.Left -= SIDE_W + OFFSET;
fieldsRect.Right -= SIDE_W + OFFSET;*/

You could have:

#ifdef SHOW_MAP
 fieldsRect.Left += SIDE_W + OFFSET;
 fieldsRect.Right += SIDE_W + OFFSET;
 WriteConsoleOutput(out,map,fieldsGrid,origin,&fieldsRect);
 fieldsRect.Left -= SIDE_W + OFFSET;
 fieldsRect.Right -= SIDE_W + OFFSET;
#endif

Function length

Your main is about 180 lines long. This is quite long, and whilst it's not always the case, there is likely to be some aspects of it that could be broken up into separate functions to make the intent and flow of the code clearer. Generally speaking, if I get a method that is much more than a screen full of code (80 lines) I start considering if the code needs broken up more.

labels

I'm not totally against labels, but if you need to use them, then it can be a good indicator as to where there is a distinct responsibility in your code. For example, you have the label start:. You jump to this if the player says they want to play again. If the code had been broken up a bit, this label wouldn't have been necessary:

do {
 playGame();
}
while(userWantsToPlayAgain());

map

map is a collection in the STL, so I would tend to avoid using it as a variable name. I'd also consider using a two dimensional array to represent the play area, rather than a 1 dimensional array. That way instead of doing the hard work yourself:

minefield[i + j * SIDE_W].Char.AsciiChar = ' ';

You can get the compiler to help you out:

minefield[i][j].Char.AsciiChar = ' ';

One letter variables

One letter iterators are ok, however when you start getting multiple iterators in the same method it may be worth giving them proper names so that it's more obvious what's going on. This is far from transparent:

for (k = j - 1; k <= j + 1; k++)
{
 for (l = i - 1; l <= i + 1; l++)
 {
 if ((unsigned char)map[l * 2 + k * 2 * SIDE_W].Char.AsciiChar == BOMB_CH && l >= 0 && 0 <= k && l < WIDTH && k < HEIGHT)
 {
 minesSurrounding++;
 }
 }
}

Next steps

Consider allowing the user to set the difficulty (and varying the number of mines in the minefield accordingly), rather than having it hard coded.

• EshelBM: that was really helpful
• EshelBM: But I'm quite sure that the array "minefield" has to be one-dimensional in order to pass it properly to the function 'WriteConsolOutput'.
• forsvarir: @programmer I don't know if it's guaranteed by the spec (and I haven't tested it with WriteConsoleOutput), but in my experience simple 2d arrays occupy the same space as 1d arrays. Have a look at this where I initialise a 1d array, copy the entire buffer over a 2d array, then output the counter from the 2d array.

Answer 2

This makes me uneasy:

//No change allowed
#define SIZE SIDE_H * SIDE_W
#define SIDE_W (WIDTH * 2 - 1) //the horizontal side of the grid of the minefield (buttons + space between buttons)
#define SIDE_H (HEIGHT * 2 - 1) //the vertical side of the grid of the minefield (buttons + space between buttons)

using a macro before defining it, even if it works, is weird. Instead use:

//No change allowed
#define SIDE_W (WIDTH * 2 - 1) //the horizontal side of the grid of the minefield (buttons + space between buttons)
#define SIDE_H (HEIGHT * 2 - 1) //the vertical side of the grid of the minefield (buttons + space between buttons)
#define SIZE SIDE_H * SIDE_W

or even better

const int SIDE_H = HEIGHT * 2 -1;
const int SIDE_W = WIDTH * 2 - 1;
const int SIZE = SIDE_H * SIDE_W;

so you don't use macros in C++ code.

• Brian Sutherland: Why do you suggest to not use macros in c++ code?
• pm100: because macros are invisible to the compiler and thus break type safety, debugging, compiler diagnostics,.... Almost all the common uses of macros are no longer needed, See scott myers effective c++ item #1
• Ped7g: And if this is supposed to be C++, why not to use constexpr instead of const in this particular case.
• pm100: because i dont know what constexpr does :-)
• EshelBM: yeah I don't know why i did that...

Answer 3

This question is already quite old. Still there's a lot more to suggest for improvements.

Here are some suggestions not mentioned yet.

1. Don't use #define for constants: This:

   //read only variables - can be changed.
   #define NUM_OF_BOMBS 60 //HAS TO BE LESS THAN 100 AND LESS THAN HEIGHT * WIDTH
   #define HEIGHT 13 //the amount of actual "buttons" in the height of the grid
   #define WIDTH 25 //the amount of actual "buttons" in the width of the grid
   #define OFFSET 3 //how far away is the minefield from the sides of the screen
   #define OFFSET_FLAG_W 20 //how far away is the flags counter from the left of the screen
   #define OFFSET_FLAG_H 1 //how far away is the flags counter from the top of the screen
   #define BUTTON_CH 219 //this is the ascii value of a blank square - character: '█'
   #define BOMB_CH 15 //'☼'
   //No change allowed
   #define SIZE SIDE_H * SIDE_W
   #define SIDE_W (WIDTH * 2 - 1) //the horizontal side of the grid of the minefield (buttons + space between buttons)
   #define SIDE_H (HEIGHT * 2 - 1) //the vertical side of the grid of the minefield (buttons + space between buttons)
   #define FLAGS_DISP_SIZE 4
   //colors (b= background, f=foregruond):
   #define GRAY_F FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN
   #define WHITE_F GRAY_F | FOREGROUND_INTENSITY
   #define GRAY_B BACKGROUND_BLUE | BACKGROUND_RED | BACKGROUND_GREEN
   #define WHITE_B GRAY_B | BACKGROUND_INTENSITY
   

   The first step here is to realize that we now have true constants in C++ without using the preprocessor. Why bother? Well the preprocessor doesn't follow the rules of C++ always. For more detail see this: https://stackoverflow.com/questions/42388077/constexpr-vs-macros

   Anyway your code then becomes this:

   //read only variables - can be changed.
   constexpr auto num_of_bombs{ 60 }; //HAS TO BE LESS THAN 100 AND LESS THAN HEIGHT * WIDTH
   constexpr auto height{ 13 }; //the amount of actual "buttons" in the height of the grid
   constexpr auto width{ 25 }; //the amount of actual "buttons" in the width of the grid
   constexpr auto offset{ 3 }; //how far away is the minefield from the sides of the screen
   constexpr auto offset_flag_w{ 20 }; //how far away is the flags counter from the left of the screen
   constexpr auto offset_flag_h{ 1 }; //how far away is the flags counter from the top of the screen
   constexpr auto button_ch{ 219 }; //this is the ascii value of a blank square - character: '█'
   constexpr auto bomb_ch{ 15 }; //'☼'
   //No change allowed
   constexpr auto side_w(width * 2 - 1); //the horizontal side of the grid of the minefield (buttons + space between buttons)
   constexpr auto side_h(height * 2 - 1); //the vertical side of the grid of the minefield (buttons + space between buttons)
   constexpr auto size{ side_h * side_w };
   constexpr auto flags_disp_size{ 4 };
   //colors (b= background, f=foregruond):
   constexpr auto gray_f{ FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN };
   constexpr auto white_f{ gray_f | FOREGROUND_INTENSITY };
   constexpr auto gray_b{ BACKGROUND_BLUE | BACKGROUND_RED | BACKGROUND_GREEN };
   constexpr auto white_b{ gray_b | BACKGROUND_INTENSITY };
   

2. State intent directly in Code. For this only one example. Consider this code we have now:

   constexpr auto button_ch{ 219 }; //this is the ascii value of a blank square - character: '█'
   constexpr auto bomb_ch{ 15 }; //'☼'
   

   What does 219 mean? You said it in the comment. It would be better to say it in the code like this:

   constexpr auto button_ch{ '█' };
   constexpr auto bomb_ch{ '☼' }; 
   

   Now nobody needs to read the comments any more. Unfortunately these signs are not portable so I switched them to some standard signs on the keyboard:

   constexpr auto button_ch{ '#' };
   constexpr auto bomb_ch{ '*' };
   

3. Limit your line width. Why? To open two source files next to each other without scrolling. Also it stresses the eye to scroll to the right as well. For example this:

   int width = offset_flag_w + flags_disp_size + 1 > side_w + offset * 2 ? offset_flag_w + flags_disp_size + 1 : side_w + offset * 2;
   

   Isn't that better:

   int width = offset_flag_w + flags_disp_size + 1 > side_w + offset * 2 ? 
    offset_flag_w + flags_disp_size + 1 : 
    side_w + offset * 2;
   

   There are tools that can show you how long a line can be. Common limits are 80 or 100 chars. I personally stick to 80. I think it also forces you to write cleaner code.

4. Limit your function size. You already made functions; good. The bad part is they are still too long. For example your main function. Is it easy to follow how the program flows there? I think not. There are many nasty details to follow the function. You should consider hiding them into their own small functions. I would break them into many small functions which each only do one thing. Generally a function should fit on one screen without scrolling. Otherwise the function is probably too big and does too many tasks.

I will here later to add more when I have the time...

• EshelBM: Thanks! very insightful. I agree that constexpr are better, and now I will start to use them. Until now, I always thought #defines are better, since I was only told that they make the program more difficult to reverse-engineer, because they don't occupy memory.