Minesweeper in C++

Question

I would like general ideas on improvement for this implementation of Minesweeper I wrote in C++. I am not really good in OOP yet, so I want ideas about how I can refactor the code by adding objects (or structs) without it getting too bulky.

/*Minesweeper C++
Written by:-Teodor D.*/  

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

/*Rules:
The player enters 'o' , then enters value of i and j to open cell[i][j].
Enter 'f' ,then enter value of i and j to place a flag on cell[i][j] */

using namespace std;

void reveal(int, int);  /// reveals a cell with given coordinates 
void create_mine_positions(); 
void cell_number(); //increases the number of a cell with 1
void create_table(); //creates the game table
void open_cell(); // opens a cell
void game(); 
void print_table(char); // prints the game table

char table[10][10]; //the game table visible ot the player
char table_mine_positions[10][10]; //table with the positions of the mines and the number of each cell
char symbol; //the input symbol, it can be 'o' or f'
int flag_counter=0;
int mines_flagged_counter=0;
bool end_game_lose=false;
time_t time_since_epoch = time(0);
time_t game_time;

void cell_number(int i,int j)
{
if(i>=0&&i<10&&j>=0&&j<10&&table_mine_positions[i][j]!='X')
table_mine_positions[i][j]++;
}

void create_mine_positions()
{
int counter=0;
srand(time(NULL));

for(int i=0;i<10;i++)
        for(int j=0;j<10;j++)
            table_mine_positions[i][j]='0';

        int i=0;
        int j=0;
 while(counter<10)
    {
        int i=rand()%10;
        int j=rand()%10;
        if(table_mine_positions[i][j]=='0'){
            table_mine_positions[i][j]='X';

            cell_number(i-1,j);
            cell_number(i+1,j);
            cell_number(i,j-1);
            cell_number(i,j+1);
            cell_number(i-1,j-1);
            cell_number(i-1,j+1);
            cell_number(i+1,j-1);
            cell_number(i+1,j+1);
            counter++;
        }
    }
}

void create_table()
{
    for(int i=0;i<10;i++)
        for(int j=0;j<10;j++)
            table[i][j]='*';
}

void print_table(char arr[10][10])
{
    cout<<"    ";
    for(int i=0;i<10;i++)
        cout<<setw(3)<<i;

    cout<<endl<<"  ";
    for(int i=0;i<32;i++)
        cout<<"_";
        cout<<endl;

    for(int i=0;i<10;i++){
            cout<<setw(3)<<i<<"|";
        for(int j=0;j<10;j++)
            cout<<setw(3)<<arr[i][j];
            cout<<endl;
    }
}

void open_cell()
{
int i,j;

do
cin>>i>>j;
while(i<0||i>9||j<0||j>9);

if(table_mine_positions[i][j]=='X')
    {
        table[i][j]='X';
        end_game_lose=true;

        for(int i=0;i<10;i++)
            for(int j=0;j<10;j++)
                if(table_mine_positions[i][j]=='X')
                    table[i][j]='X';
    }
    else
        reveal(i,j);
}

void place_or_remove_flag()
{
int i,j;
do
cin>>i>>j;
while(i<0||i>9||j<0||j>9);

if (table[i][j]=='*')
    {
    table[i][j]='F';
    flag_counter++;

    if(table_mine_positions[i][j]=='X')
        mines_flagged_counter++;
    }

else if (table[i][j]=='F')
    {
    table[i][j]='*';
    flag_counter--;
    if(table_mine_positions[i][j]=='X')
        mines_flagged_counter--;
    }
}

void input_symbol()
{
   cin>>symbol;
    switch (symbol){
    case 'o' : open_cell(); break;
    case 'f' : place_or_remove_flag(); break;
    default  : input_symbol();
    }
}

void reveal(int i,int j)
{
if (table[i][j]=='*'&&table_mine_positions[i][j]!='X'&&i>=0&&i<10&&j>=0&&j<10)
    {
    table[i][j]=table_mine_positions[i][j];

    if(table_mine_positions[i][j]=='0')
        {
        reveal(i,j-1);
        reveal(i,j+1);
        reveal(i-1,j-1);
        reveal(i+1,j-1);
        reveal(i+1,j+1);
        reveal(i-1,j+1);
        reveal(i-1,j);
        reveal(i+1,j);
        }
    }
}

bool end_game_win_check()
{
    if(flag_counter==10&&mines_flagged_counter==10)
    return 1;
    else
    return 0;
}

void game()
{
    create_table();
    create_mine_positions();

    while(!end_game_lose&&!end_game_win_check())
    {
    game_time=time(0);
    print_table(table);
    cout<<endl<<"Flags:"<<flag_counter<<endl;
    cout<<"Time:"<<game_time-time_since_epoch<<endl;
    input_symbol();
    }

    if(end_game_lose){
    print_table(table);
    cout<<endl<<"GAME OVER"<<endl;
    }

    if(end_game_win_check())
    cout<<"Time to complete:"<<game_time-time_since_epoch<<endl;
    cout<<endl<<"YOU WIN!"<<endl;
}

int main()
{
cout
<<"Rules:"
<<endl<<"Enter 'o' , then enter value of i and j to open cell[i][j]."
<<endl<<"Enter 'f' ,then enter value of i and j to place "
<<"or remove flag on cell [i][j]."
<<endl<<endl;

game();

return 0;
}

Answer 1

Some initial thoughts...

• Consider labelling your axis, or complying with a known standard. When I'm looking at coordinates, I'm thinking (x,y) for horizontal, then vertical. You're asking for i,j coordinates, for vertical, then horizontal. This is likely to be a bit confusing to the user the first time they play. You're also using i,j in your code, again, I'd tend to use x,y and have them apply in the expected way.

• Your formatting is quite erratic, if it looks like that in your IDE, I'd want to fix it. Consistent bracing/indentation makes code a lot easier to read. If it's OK in your development environment, then usually you'd just copy and paste the whole of your code into the question, select it and click the code {} icon.

• It's generally considered good practice to brace your statements. Consider this code:

  do
  cin>>i>>j;
  while(i<0||i>9||j<0||j>9);
  

  It's perfectly valid, however, with no indentation or braces at first glance it looks a bit like an infinite while loop.

• Users are awfully unreliable, even those with the best intentions will eventually make a mistake. What happens if instead of putting 'o 3 4', the user enters 'o e 4' by mistake? The loop above is unable to resolve the problem, so loops forever. Doing something like this (untested) would help to resolve the bad input, although obviously you might want to prompt the user again...:

  do {
      cin >> i >> j;
      if (cin.fail()) 
      {
          cin.clear(); 
          cin.ignore();
      }
  }
  while (i<0 || i>9 || j<0 || j>9);
  

• end_game_win_check - At the moment, for the user to win, they have to have flagged all ten of the mines. I sometimes used to play minesweep by not flagging any of the mines and just trying to clear all the squares that didn't have mines on them. It seems like if the user has cleared cell that doesn't have a mine they should also have won...

• OO - As you've said, you've not really used any classes or structs. I would suggest that a good starting point would be to create a class and move your global variables into the class as members. Move your existing functions into the class as member functions. Then consider which functions actually need to be public and which can be private. The next step might be to try and extract the user interaction, particularly input out of the class, so that you're exposing operations to open_cell(x,y) where x+y have come from another class that is responsible for asking the user for moves.