Battleships console game

I have built a light version of a battleships program and would love for anyone to critique it. The game does not feature OOP, AI nor multiple boards. I aimed more for functionality than correct variable names and other related things just to get it working temporarily so I'm sorry if some things are named badly.

The player will only simply continue to guess on a singular board to see if their shots hit the target amount of ship until the game is completed.

I attempted to make the program adjust dynamically to what difficulty the player has set. For example, if the player were to choose easy, then the game board has 2x2 squares. Medium has 4x4 and Hard has 8x8.

```
// Battleships.cpp : Defines the entry point for the console application.
//
#include "stdafx.h"
#include
#include
#include
#include

namespace game
{
int rows = 0;
int columns = 0;
int maxNumbShips = 0;

const char WATER = '~';
const char HIT = 'X';
const char MISS = '/';
const char SHIP = 'S';
const char EMPTY = ' ';
}

void initBoard(std::vector>& playingBoard);
void displayBoard(std::vector> playingBoard);
void initShips(std::vector>& ships);
bool isGameOver(int shipsDestroyed);
void shoot(std::vector>& playingBoard, std::vector>& ships, int& shipsDestroyed, int& turns);
int enterRowPosition();
int enterColPosotion();
void selectDifficulty();

int main()
{
std::vector> playingBoard;
std::vector> ships;

int shipsDestroyed = 0; //How many ships have been destroyed by the player
int turns = 0;

selectDifficulty();
initBoard(playingBoard);
initShips(ships);
displayBoard(playingBoard);

while (!isGameOver(shipsDestroyed))
{
shoot(playingBoard, ships, shipsDestroyed, turns);
displayBoard(playingBoard);
}

std::cout > barn;
return 0;
}

void initBoard(std::vector>& playingBoard)
{
std::vector tempVector;
for (int i = 0; i >& ships)
{
std::vector tempVector;
for (int i = 0; i > playingBo

Remove unnecessary headers

#include "stdafx.h"
#include 

You don't need both of these headers. The first is a Windows-only header (which limits portability of your application) and the other is only used on a single instance that is completely unimportant to the execution of the program. If you don't want the console to quit after the game ends, just do:

char c;
cin >> c;

That way, you're avoiding including a huge header for such a trivial task.

Remove function prototypes

For such a simple program like this, function prototypes are not necessary; we generally like to keep function prototypes around only in header files (for documentation and encapsulation purposes). Moreover, they introduce a new source of maintenance for you if your function signatures ever change. Just move the main function to the bottom of the file and keep all the other functions above it.

Think about making this OOP

As you said, you were in a rush making this but I would strongly recommend revisiting this and making at least the board a class with some good setters/getters.

Code Redundancy / Game Design

void initBoard(std::vector>& playingBoard)
{
    std::vector tempVector;
    for (int i = 0; i >& ships)
{
    std::vector tempVector;
    for (int i = 0; i < game::rows; i++)
    {
        tempVector.push_back(game::EMPTY);
    }

    for (int i = 0; i < game::columns; i++)
    {
        ships.push_back(tempVector);
    }

// ...
}

This is the exact same code repeated twice. Factor it out as a helper function then call it to obtain a ships vector and a playingBoard vector. Here is an example (since C++11):

std::vector> CreateBoard(char fill_value)
{
    std::vector vec{game::rows, fill_value};
    return {game::columns, vec};
}

This constructs a vector of game::rows chars that are initialized to fill_value. Then it returns a vector of those that is game::columns big. You also avoid the need for awkward references in the parameters this way.

In initShips

std::vector> initShips()
{
    auto ships = CreateBoard(game::EMPTY);

    /// randomly put ships in ships :)

    return ships;
}

Now, you have no need for the initBoard function either. In main, you can just do:

auto playingBoard = CreateBoard(game::WATER);
auto ships = initShips();

Simplify game logic

srand(time(NULL));
bool emptyPosition = false;

for (int ship = 0; ship < game::maxNumbShips; ship++)
{
    do
    {
        int x = rand() % game::rows;
        int y = rand() % game::columns;

        if (ships[x][y] == game::SHIP)
        {
            emptyPosition = false;
        }
        else
        {
            ships[x][y] = game::SHIP;
            emptyPosition = true;
        }
    } while (!emptyPosition);
}

Here, you already have emptyPosition set to false. There's no need to reassign it to that. Just have the value change if a certain criterion is met:

for (int ship = 0; ship < game::maxNumbShips; ship++)
{
    do
    {
        int x = rand() % game::rows;
        int y = rand() % game::columns;

        if (ships[x][y] != game::SHIP)
        {
             emptyPosition = true;
             ships[x][y] = game::SHIP;
        }
    } while (!emptyPosition);
}

Unnecessary Copy

Your displayBoard function should be taking in the vector by const reference as you don't need a copy of the board and you're not modifying it.

Error Handling

Think about what happens if the user enters the number "4" on the difficulty selection menu. As it is, the game will proceed and not do anything. This isn't how a game is meant to flow. I would make sure that the user entered a valid choice at that menu and maybe provide an option to quit if they really want to exit the program.

StackExchange Code Review Q#116883, answer score: 2