Revision b6fb9b72-0ca7-4ea2-af1f-dbb2c2f49b5d - Code Review Stack Exchange

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 intend 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 anymore. 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 aswell. 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 who 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 who best 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...