I cannot check all because I use a phone at the moment, but here are some quick observations I saw skimming thru the code:
Never call main ()
-It is not defined in C++ what happens if you call main from another function. It is undefined behavior.
You could instead put your while (1) loop in a function and call it instead of main.
Use of C++ Features
-Consider using std::array or std::vector instead of using plain C style arrays.
Formating
-Formatting seem to be off in some places. Add a space between functions and don’t put several instructions on one line.
e.g.
if (win()) {std::cout << "Player 2 wins!\n";break;}
++count;
Should become:
if (win()) {
std::cout << "Player 2 win\n";
break;
}
++count;
Also you should avoid having to long lines. Get used to break the code after max 80 lines. It has the advantage that you can view two source codes next to each other.
for example make this:
if (pos[w_combinations[i][0]] == pos[w_combinations[i][1]] && pos[w_combinations[i][0]] == pos[w_combinations[i][2]])
{
return true;
break;
}
Into this:
if (pos[w_combinations[i][0]] == pos[w_combinations[i][1]]
&& pos[w_combinations[i][0]] == pos[w_combinations[i][2]])
{
return true;
break;
}
or this
void printMatrix() {
std::cout << "|" ; check(pos[0]) ; check(pos[1]) ; check(pos[2]);std::cout << "\n";
std::cout << "|" ; check(pos[3]);check(pos[4]) ; check(pos[5]);std::cout << "\n";
std::cout << "|" ; check(pos[6]) ; check(pos[7]) ;
It’s very important for code to be easy to read. You may be only write a program once but most of the time you read it while debugging or maintaining it. If the code is good formatted this task becomes a lot easier.
Avoid redundancy
In you’re while loop you use the same instructions except the messages for player 1 and 2.
Make them a function with parameters for the messages. This way you make sure you handle both players the same.
Avoid global variables
Don’t use global variables they make the code hard to maintain. They can lead to obscure bugs.
Consider passing them between you’re functions or even better check out OOP and work with a class who stores the state of the Board.
Functions should do one thing
You’re function check () should only return the string not also print it. It should become this:
std::string check (int x)
{
if ( x == 10) {
return "X|";
}
else if (x == 11) {
return "O|";
}
return std::to_string (X)
}
This function now gets called and the string is directly printed. No need to change anything in the call code.
Also consider using switch instead of a long if else chain if there are many cases.
Calls to std::cout
No need to call std::cout several times. Just do this :
std::cout << '|' + check(pos[0]) + check(pos[1]) + check(pos[2]) + '\n';
Single chars should be quoted ' ' not " " to avoid getting a more expensive string use instead of a char.
Handle invalid input
Try to enter something which is not a the expected number. What happens? The program probably loops endless or crashes because you read no int.
You should handle this case.
One approach would be reading the in put as a std::string and check this string if it is a digit.
If it is not prompt an invalid input and let the user enter again.
But wait. you can also break the program if it is an int but not in range. Handle also wrong range. for example when you input 1234 the program will crash currently as well. You could do all this in a function:
int get_input();
So you can write:
int x = get_input();
Use descriptive names
Function names like Check
or Input
are very mysterious to me about what they are doing.
The same goes for variable names like x, y or w_combinations.
Try to self document youre code by using clear names which describe what the variable or function does.
Ask yourself if you check this code in 1 or 2 months yourself if you still can get what the program does. With good naming’s and a clear structure it becomes a lot easier.
Use C++ Features like Classes
Youre code looks very C like. Make more use of C++ features like classes etc.
I rewrote the program with classes to demonstrate a more C++ Style as an inspiration (maybe I should also post it for review?).
Don't get overwhelmed it takes a lot of time and practice to get into C++...
tic_tac_toe.h
#ifndef TIC_TAC_TOE_020120180815
#define TIC_TAC_TOE_020120180815
#include <array>
#include <string>
namespace tic_tac_toe
{
class TicTacToe final{
public:
TicTacToe() = default;
~TicTacToe() = default;
// delete copy and move mechanism, we don't want to
// copy a running game
TicTacToe(const TicTacToe&) = delete;
TicTacToe(TicTacToe&& other) = delete;
TicTacToe& operator=(const TicTacToe& other) = delete;
TicTacToe& operator=(TicTacToe&& other) = delete;
void print_state_of_board() const;
bool draw(int field);
bool board_full() const;
bool player1_win() const
{
return check_win_condition(FieldState::player1);
}
bool player2_win() const
{
return check_win_condition(FieldState::player2);
}
private:
enum class FieldState {
empty,
player1, // X
player2, // O
};
bool check_win_condition(FieldState state) const;
char field_state_to_char(FieldState state) const;
std::array<FieldState, 9> m_board{ FieldState::empty };
bool m_player1_active{ true };
static constexpr char m_player1_token{ 'X' };
static constexpr char m_player2_token{ 'O' };
};
int get_user_input(const std::string& user_message);
void play_game(); // main routine to run the game logic;
} // namespace tic_tac_toe
#endif
tic_tac_toe.cpp
#include "tic_tac_toe.h"
#include <algorithm> // std::find
#include <cctype> // std::stoi
#include <iostream>
#include <vector>
namespace tic_tac_toe
{
void TicTacToe::print_state_of_board() const
/*
Print the board. e.g:
|X| |O|
| |X| |
|O| | |
*/
{
for (auto i = 0; i < m_board.size(); ++i) {
if (i % 3 == 0 && i != 0) {
std::cout << "|\n";
}
auto token = field_state_to_char(m_board.at(i));
std::cout << '|' << token;
}
std::cout << "|\n";
}
bool TicTacToe::draw(int field)
/*
Tries to draw the next symbol in the field.
Each time the function is called the player is changed.
The user input has to be done out side. This way also a bot
could play the game.
If the selected field can not be set because its already
occupied by player1 or player2 or out of range the return
value becomes false
*/
{
if (field < 1 || field > m_board.size() ||
m_board.at(field - 1) != FieldState::empty) {
return false;
}
if (m_player1_active) {
m_board.at(field - 1) = FieldState::player1;
m_player1_active = false;
}
else { // player 2 active
m_board.at(field - 1) = FieldState::player2;
m_player1_active = true;
}
return true;
}
bool TicTacToe::board_full() const
/*
search for a empty field in the board
indicating that board is full if no empty field available.
*/
{
auto it = std::find(
m_board.begin(), m_board.end(), FieldState::empty);
return it == m_board.end();
}
bool TicTacToe::check_win_condition(FieldState state) const
{
constexpr std::array<std::array<int, 3>, 8> combinations =
{
std::array<int, 3>{0,1,2},
std::array<int, 3>{3,4,5},
std::array<int, 3>{6,7,8},
std::array<int, 3>{0,3,6},
std::array<int, 3>{1,4,7},
std::array<int, 3>{2,5,8},
std::array<int, 3>{0,4,8},
std::array<int, 3>{2,4,6}
};
for (const auto& combination : combinations) {
if (m_board.at(combination[0]) == state &&
m_board.at(combination[1]) == state &&
m_board.at(combination[2]) == state) {
return true;
}
}
return false;
}
char TicTacToe::field_state_to_char(FieldState state) const
{
if (state == FieldState::player1) {
return m_player1_token;
}
if (state == FieldState::player2) {
return m_player2_token;
}
return ' ';
}
int get_user_input(const std::string& user_message)
{
while (true) {
std::cout << user_message;
std::string input;
std::cin >> input;
/*
If input is not an integer the stoi function will raise
an exception. We use this to determine if the input was
an int
*/
try {
return std::stoi(input);
}
catch (std::invalid_argument&) {
std::cout << "\nInput is not a number. Try again:";
}
}
}
void play_game()
/*
Main routine to play the game with 2 players
*/
{
while (true) {
TicTacToe game;
bool player1_active{ true };
while (!game.board_full() &&
!game.player1_win() && !game.player2_win()) {
game.print_state_of_board();
std::string user_message;
if (player1_active) {
user_message = "1[X]:";
}
else { // player2 active
user_message = "2[O]:";
}
if (!game.draw(get_user_input(user_message))) {
std::cout << "\nInvalid! Try again: \n";
}
else {
player1_active = !player1_active;
}
}
game.print_state_of_board();
if (game.player1_win()) {
std::cout << "Player 1 wins!\n";
}
else if (game.player2_win()) {
std::cout << "Player 2 wins!\n";
}
else {
std::cout << "Draw!\n";
}
int choice{};
while (true) {
choice = get_user_input(
"Play again[Yes = 1, No = 0]: ");
if (choice == 0) {
return;
}
if(choice == 1) {
break;
}
}
}
}
} // namespace tic_tac_toe
main.cpp
#include "tic_tac_toe.h"
#include <iostream>
int main()
try {
tic_tac_toe::play_game();
}
catch (std::runtime_error& e) {
std::cerr << e.what() << "\n";
std::getchar();
}
catch (...) {
std::cerr << "unknown error " << "\n";
std::getchar();
}