C# Tic-Tac-Toe Game

Question

Basically I am made a class that settles most of the back-end work of a tic-tac-toe game.
I would like to have remarks about my general coding style and possible optimizations.

using System;

namespace TicTacToe_Game
{
    public class Game
    {
        //The game's board itself.
        //If the element is 0, the tile is unmarked.
        //If the element is 1, the tile is marked by player1.
        //If the element is 2, the tile is marked by player2.
        private byte[,] board = new byte[3,3];
        //Used to check which player's turn it is.
        private bool player1_turn = true;
        private bool gameWon = false;

        public Game() {
            //Prepares the game board.
            for (int i = 0; i < 3; ++i) {
                for (int n = 0; n < 3; ++n) {
                    board[i, n] = 0;
                }
            }
        }

        public byte[,] GetBoard() {
            return board;
        }

        /// <summary>
        /// Resets the current game.
        /// </summary>
        public void ResetGame() {
            player1_turn = true;
            gameWon = false;
            //Resets the game board.
            for (int i = 0; i < 3; ++i)
            {
                for (int n = 0; n < 3; ++n)
                {
                    board[i, n] = 0;
                }
            }
        } 

        private byte UpdateBoard(byte x, byte y) {
            //1 for win, 2 for no win, 0 for error.
            byte isWinningMove = 2;

            byte marking;
            if (player1_turn)
            {
                marking = 1;
            }
            else {
                marking = 2;
            }

            //If the marked tile was not occupied.
            if (board[x, y] == 0)
            {
                board[x, y] = marking;

                int counter = 0;

                //Check each horizontal row for wins.
                for (int i = 0; i < 3 && isWinningMove != 1; ++i)
                {
                    for (int n = 0; n < 3; ++n)
                    {
                        if (board[i, n] == marking) {
                            ++counter;
                        }
                    }
                    //If there is a row of equivalent markings.
                    if (counter == 3)
                    {
                        //The player wins.
                        isWinningMove = 1;
                    }
                    counter = 0;
                }

                //Check each verticle column for wins.
                for (int i = 0; i < 3 && isWinningMove != 1; ++i)
                {
                    for (int n = 0; n < 3; ++n)
                    {
                        if (board[n, i] == marking)
                        {
                            ++counter;
                        }
                    }
                    //If there is a column of equivalent markings.
                    if (counter == 3)
                    {
                        //The player wins.
                        isWinningMove = 1;
                    }
                    counter = 0;
                }

                //Check for diagonals.
                if ((board[0, 0] + board[1, 1] + board[2, 2]) == (marking * 3))
                {
                    isWinningMove = 1;
                }
                else if ((board[0, 2] + board[1, 1] + board[2, 0]) == (marking * 3)) {
                    isWinningMove = 1;
                }
            }
            //If the requested tile to be mark was already occupied.
            else {
                //Returns an error code.
                isWinningMove = 0;
            }

            return isWinningMove;
        }

        public bool NewMove(byte row, byte column) {
            //If the user made a move when the game is already over.
            if (gameWon) {
                //Resets the current game first.
                ResetGame();
            }

            //If the new move is not a valid move.
            if (row > 2 || column > 2) {
                throw new InvalidOperationException();
            }

            byte status = UpdateBoard(row, column);
            bool result;
            if (status == 0)
            {
                throw new InvalidOperationException();
            }
            else if (status == 1)
            {
                gameWon = true;
                result = true;
            }
            else {
                result = false;
            }

            //Changes the player turn for the next move.
            if (player1_turn)
            {
                player1_turn = false;
            }
            else
            {
                player1_turn = true;
            }

            return result;
        }

        public bool NewMove(byte tileNumber)
        {
            //If the user made a move when the game is already over.
            if (gameWon)
            {
                //Resets the current game first.
                ResetGame();
            }

            if (tileNumber == 0 || tileNumber > 9) {
                throw new InvalidOperationException();
            }

            byte row = 0, column = 0;

            for (byte i = 1, x = 0, y = 0; i < 10; ++i) {
                if (i == tileNumber) {
                    row = x; column = y;
                    break;
                }
                ++y;
                if (y == 3) {
                    ++x;
                    y = 0;
                }
            }

            /*
             Tile number is visualized as this way:
             1  2   3
             4  5   6
             7  8   9
             */

            byte status = UpdateBoard(row, column);
            bool result;
            if (status == 0)
            {
                throw new InvalidOperationException();
            }
            else if (status == 1)
            {
                gameWon = true;
                result = true;
            }
            else
            {
                result = false;
            }

            //Changes the player turn for the next move.
            if (player1_turn)
            {
                player1_turn = false;
            }
            else {
                player1_turn = true;
            }

            return result;
        }
    }
}

Answer 1

//The game's board itself.
//If the element is 0, the tile is unmarked.
//If the element is 1, the tile is marked by player1.
//If the element is 2, the tile is marked by player2.
private byte[,] board = new byte[3,3];

You did well by commenting the states of the board elements, however you could eliminate the comments by using an enum. That will increase readability, maintainability and makes your comments obsolete:

enum BoardStates
{
    Unassigned,
    X,
    O
}

private BoardStates[,] board = new BoardStates[3,3];

---

Make sure to use a consistent naming scheme. You used a mixture of camelCase (gameWon) and snake_case (player1_turn). Stick with one and your code is easier to read. It would be even better to use the C# naming convention.

---

Additionally try to separate your code further. You could use a bool CheckRow(rowIndex) method to check if a player has already won. The same applies for a bool CheckColumn(int columnIndex) method.

If you are able to implement the check in one method it is even better, but i would only recommend doing this if the method is still short and easy to read. Otherwise it is fine to go with two methods.

Answer 2

Private fields usually take the form of private int _foo;

I'd suggest switching to that naming convention, however you have remained consistent, so that is good.

In your constructor you logically create the board, however, upon game reset, the same logic is invoked. I would highly recommend putting your board construction into a ConstructBoard() method, so this can be used;

public void ConstructBoard()
{
  for (int i = 0; i < 3; ++i)
      for (int n = 0; n < 3; ++n)
          board[i, n] = 0;
}

Not too sure on the reasons of the GetBoard() method, as this just returns your board. You could interact with this directly. Adds a little obscurity if you ask me.

I think you'd really benefit from using const byte as well. byte isWinningMove = 2; is decorated with a comment, but no real insight as to what 2 actually means. In context, I'd probably start putting something like;

private const byte ERROR = 0;
private const byte WIN = 1;
private const byte LOSE = 2;

That way if your board ever changes or your logic changes, it's a simple switch on the constants. I'd suggest the same for the marking of the player.

Your UpdateBoard() method has quite a substantial if, with an attached else for an error state-type scenario, wherein the cell might already be occupied. I would check that before even going further into the method.

if (board[x, y] != UNMARKED) // Notice the constant
  return ERROR;

After this point you can continue your marking logic, as you are now sure there is nothing in that cell.

I'd recommend making UpdateBoard() return something other than a byte, you're not gaining much from it and it's somewhat obscure. I'd start making some kind of a response class.

private class Move
{
  public Move(bool isSuccessfulMove, bool isGameWin)
  {
    IsSuccessfulMove = isSuccessfulMove;
    IsGameWin = isGameWin;
  }
  public bool IsSuccessfulMove { get; }
  public bool IsGameWin { get; }
}

That is a very crude example, but this gives a lot more intention and tight control over the scenario of an updated board after a move/game. This would alleviate if (status == 0) from happening. As again, this means nothing to anyone else looking at your code. This would simply become if (move.IsSuccessfulMove).

You can save some lines of code when switching players.

//Changes the player turn for the next move.
if (player1_turn)
{
  player1_turn = false;
}
else
{
  player1_turn = true;
}

Could become _isPlayerOneTurn = !_isPlayerOneTurn; I also took the liberty of renaming the field as it doesn't really follow convention.

It's good to see an overload of NewMove() that allows for both a grid reference and a tile reference. But you do have repeated logic. I would separate out some to methods and reuse this logic in case that ever has to change, which I know, in a Tic-Tac-Toe game is unlikely, but this is all about practice.

Lastly, I'm a little put off by the exceptions you're throwing, why throw? This could be handled better with an error state or other concept.