Minesweeper in C++ using objects

After taking a course in C++, I have written my first program and I'm looking for some feedback both on coding style and maybe algorithmic improvements. The code can also be found at GitHub. It compiles and runs fine but is not very robust for user input.

I opted to try to emulate the game Minesweeper. My implementation is as follows:

#include "minesweeper/minesweeper.h"

int main()
{
    Minesweeper game{9};
    game.play();
}

First I have a class which I use as a 'cell':

#ifndef MINESWEEPERCELL_H_
#define MINESWEEPERCELL_H_

#include 

class MinesweeperCell
{
    bool d_isBomb = false;
    char d_state = '*';             // * or F
    std::size_t d_numBombsNear = 0;
    bool d_visited = false;

    public:
        MinesweeperCell() = default;
        void setBomb();
        bool isBomb() const;
        void setState(char const ch);
        char state() const;
        void setNumBombsNear(std::size_t const num);
        std::size_t numBombsNear() const;
        bool visited() const;
        void setVisited();
};    
#endif

The implementations of these getters and setters are trivial and hence are left out for brevity. They can be found at the GitHub project.

Next is the most important class:

```
#ifndef MINESWEEPER_H_
#define MINESWEEPER_H_

#include
#include
#include
#include

#include "../minesweepercell/minesweepercell.h"

class Minesweeper
{
std::size_t d_size; // square only so far
std::vector> d_gameBoard;
std::vector> d_bombLocations;
public:
Minesweeper(std::size_t const size = 9);
void play();
private:
void initializeBoard(); // Places randomly d_size + 1 bombs on board
void setNumBombsNear();
void checkSurrounding(std::size_t const xCoord, std::size_t const yCoord);
void showBombs();
bool processInput(char const cmd, std::size_t const xCoord, std::size_t const yCoord);
friend std::ostream &operator<<(std::ostream &out,

I see a number of things that may help you improve your code.

Do not use the .ih convention

The "one function per file" rule forces a proliferation of files, each of which require some thought to name intelligently. It turns what could be a single coherent file, say minesweeper.cpp into nine different source files, meaning that a reader of the code must look at all nine files to fully understand the class. This is not at all user friendly, and is not realistically sustainable for larger projects. I'm sure Frank Brokken is a nice guy, but this particular idea of his does no favors to new programmers in my humble opinion.

Avoid explicit path names in #includes

Generally speaking, it's better to tell your compiler where to find the #include files rather than putting it explicitly into the source code like this:

#include "minesweeper/minesweeper.h"

The only reason one would add such a thing is if there were a concern that your local include files have the same name as a standard include file and that you need both simultaneously.

Be careful with signed vs. unsigned

As you've already noted in a comment, there's no point to checking to see if a size_t variable is less than zero because size_t is an unsigned type.

Fix the missing return

The ostream &operator<< function has no return statement. It's simply missing this line:

return out;

Be aware that friends are not private

In the Minesweeper class we have this declaration:

friend std::ostream &operator<<(std::ostream &out,
                Minesweeper const &minesweeper);

It's in the private section, but it's important for you to be aware that it's not actually private. It's a freestanding function that can be called from anywhere. To avoid misleading the unwary, I always put friend declarations in the public section.

Use C++ idioms

The use of or and and instead of || and && in C++ code is supported by the standard, however, it's an anachronism that stems from a time when some keyboards didn't have | or & available. While some argue that it's more readable, you should be aware that many experienced C++ programmers (and some C++ compilers!) are confused by their use. For that reason, I recommend using && and ||.

Use static const where appropriate

The SetNumBombsNear() function starts with an int moves array declaration and definition. However, it doesn't change and isn't used outside the function, so I'd recommend instead that it should be declared static const or static constexpr since you're using C++11.

Think of the user

When the game first runs, it displays ... nothing. The user can't tell if it's running or locked up or just sulking. It would be nicer to the user to list the valid command (which I understand are f, u, p and q from reading the source) and maybe to add another command, h for "help" which would describe these commands. Also, the display is rather terse and requires the user to count squares in order to enter input. This makes for a tedious game! Better would be to either label the rows and columns or provide some other way for the user to indicate the desired move. Also, there is currently no way to actually win the game because the program does not track and compare the number of flags to the number of bombs.

Make your classes useful

The current MinesweeperCell class is not very useful. It provides no intelligence and has Java-style setters and getters. I'd recommend either making it a struct that is private within the Minesweeper class and eliminating all the setter/getter code or (better), make the class actually work, while hiding implementation details. For instance, at the moment, the user can flag an already revealed cell. If they then attempt to undo that faulty move by using the u command, the square reverts to an unrevealed state. Better would be to build intelligence into the MinesweeperCell class (which I would simply name Cell by the way) to prevent flagging a revealed square in the first place.

Hide class internals

The MinesweeperCell class has isBomb() and setBomb() member functions which is good because it allows the Minesweeper class to deal with the logic of the class rather than the internal details of the Cell class. I'd suggest that the setState and state member functions (and their associated d_state member) be eliminated in favor of setFlag and isFlagged functions (with an associated d_flagged member). This would allow the Minesweeper class to deal with the logic rather than the representation of the class. Then the only place that the Cell would need to be turned into a character is for display. For that, I'd suggest writing an operator function like this:

char operator()() const { 
    return (d_visited ? (d_isBomb ? 'B' : '0' + d_numBombsNear) : (d_flagged ? 'F' : '*'));
}

Some people find the use of the ?: operator difficult to read or un

StackExchange Code Review Q#159068, answer score: 7