Beginner C# console TicTacToe program

Question

I've been learning to program in C# for a couple months and wanted some feedback regarding a project I completed. Criticism and constructive feedback is what I'm after, so bring it on! I know there are improvements to be made, but I'm not sure how to implement them without crazy repetition. Where could I improve? Is my variable naming scheme wack? Let me know!

Console.WriteLine("Welcome to Tic-Tac-Toe!\n\n");
Console.WriteLine("Please enter your name:");
string[,] boardState = new string[3, 3] { // initialize the 3D array containing whitespace instead of X's and O's
 {" ", " ", " "}, // my logic will replace these whitespaces as the player or CPU adds to the board
 {" ", " ", " "},
 {" ", " ", " "}
 };
string nameSelection = "";
string spaceSelection = "";
string? readResult = Console.ReadLine();
if (readResult != null)
{
 nameSelection = readResult;
}
Console.Clear();
Console.WriteLine("GAME READY:\n\npress Enter to continue:");
Console.ReadLine();
Random roll = new Random();
int playerSelection = roll.Next(1, 3); // selecting who goes first in the game
if (playerSelection == 1)
{
 Console.WriteLine("CPU goes first!\n\nPress Enter");
 Console.ReadLine();
 while (GameWon(boardState) == false) // main game loop if the cpu goes first
 {
 CPUmove(boardState);
 DisplayGameBoard(boardState);
 GetSelection();
 // boardState changes depending on the coordinates gotten from the ParseSelection array
 boardState[ParseSelection(spaceSelection)[0], ParseSelection(spaceSelection)[1]] = "X";
 DisplayGameBoard(boardState);
 }
}
else if (playerSelection == 2)
{
 Console.WriteLine($"{nameSelection} goes first!\n\nPress Enter");
 Console.ReadLine();
 while (GameWon(boardState) == false) // main game loop if the player goes first
 {
 DisplayGameBoard(boardState);
 GetSelection();
 boardState[ParseSelection(spaceSelection)[0], ParseSelection(spaceSelection)[1]] = "X";
 DisplayGameBoard(boardState);
 CPUmove(boardState);
 }
}
Console.WriteLine($"\n\nGame Over!"); // i'm not sure how to announce who won...not without a lot of repetition
Console.WriteLine("\n\nPress Enter to end program.");
Console.ReadLine();
void CPUmove(string[,] boardState) // method to make the cpu claim a space, was originally going to be a random
 // seleciton of difficulty but im not sure how to do that.
{
 bool moved = false;
 while (moved == false)
 {
 Random roll = new Random();
 int difficultyRoll = 0;
 switch (difficultyRoll)
 {
 case 0:
 {
 if (boardState[1, 1] == " ")
 {
 boardState[1, 1] = "O";
 moved = true;
 }
 else if (boardState[2, 0] == " ")
 {
 boardState[2, 0] = "O";
 moved = true;
 }
 else if (boardState[0, 2] == " ")
 {
 boardState[0, 2] = "O";
 moved = true;
 }
 else if (boardState[2, 2] == " ")
 {
 boardState[2, 2] = "O";
 moved = true;
 }
 else if (boardState[0, 0] == " ")
 {
 boardState[0, 0] = "O";
 moved = true;
 }
 else if (boardState[1, 2] == " ")
 {
 boardState[1, 2] = "O";
 moved = true;
 }
 else if (boardState[1, 0] == " ")
 {
 boardState[1, 0] = "O";
 moved = true;
 }
 else if (boardState[2, 1] == " ")
 {
 boardState[2, 1] = "O";
 moved = true;
 }
 else if (boardState[0, 1] == " ")
 {
 boardState[0, 1] = "O";
 moved = true;
 }
 break;
 }
 }
 }
}
int[] ParseSelection(string selection) // takes the player's input and splits it into two seperate integers
{
 char[] selectionChar = selection.ToCharArray();
 switch (selectionChar[0]) // this switch determines what row to change to a char containing a corresponding number
 {
 case 'a':
 {
 selectionChar[0] = '0';
 break;
 }
 case 'b':
 {
 selectionChar[0] = '1';
 break;
 }
 case 'c':
 {
 selectionChar[0] = '2';
 break;
 }
 }
 string[] spaceIndex = { " ", " " };
 spaceIndex[0] = selectionChar[0].ToString(); // these two operations convert each literal character into a usable string
 spaceIndex[1] = selectionChar[1].ToString();
 int[] indexFinal = { 0, 0 };
 indexFinal[0] = int.Parse(spaceIndex[0]); // these two convert each string into an integer and store it in an array
 indexFinal[1] = int.Parse(spaceIndex[1]);
 return indexFinal; // returns the indexFinal array loaded with the coordinates of the selected space
}
void DisplayGameBoard(string[,] boardState) // displays the board with default values
{
 Console.Clear();
 Console.WriteLine("\t\t\t0\t\t1\t\t2");
 Console.WriteLine($"\n\ta\t|\t{boardState[0, 0]}\t|\t{boardState[0, 1]}\t|\t{boardState[0, 2]}\t|");
 Console.WriteLine("\t__________________________________________________");
 Console.WriteLine($"\n\tb\t|\t{boardState[1, 0]}\t|\t{boardState[1, 1]}\t|\t{boardState[1, 2]}\t|");
 Console.WriteLine("\t__________________________________________________");
 Console.WriteLine($"\n\tc\t|\t{boardState[2, 0]}\t|\t{boardState[2, 1]}\t|\t{boardState[2, 2]}\t|");
 Console.WriteLine("\n\nPlease pick a space 'a1' is an example space choice!");
}
string GetSelection() // basic null avoidance (i'm not sure if this is necessary) 
{
 readResult = Console.ReadLine();
 if (readResult != null)
 {
 spaceSelection = readResult.ToLower().Trim();
 }
 return spaceSelection;
}
bool GameWon(string[,] board) // checks every possible win condition for either all X's or O's (could be simpler but i'm not too sure how)
{
 if ((board[0, 0], board[1, 1], board[2, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[0, 0], board[1, 1], board[2, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[0, 0], board[0, 1], board[0, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[0, 0], board[0, 1], board[0, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[1, 0], board[1, 1], board[1, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[1, 0], board[1, 1], board[1, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[2, 0], board[2, 1], board[2, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[2, 0], board[2, 1], board[2, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[0, 0], board[1, 0], board[2, 0]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[0, 0], board[1, 0], board[2, 0]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[0, 1], board[1, 1], board[2, 1]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[0, 1], board[1, 1], board[2, 1]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[0, 2], board[1, 2], board[2, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[0, 2], board[1, 2], board[2, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else if ((board[2, 0], board[1, 1], board[0, 2]) == ("X", "X", "X"))
 {
 return true;
 }
 else if ((board[2, 0], board[1, 1], board[0, 2]) == ("O", "O", "O"))
 {
 return true;
 }
 
 else
 {
 return false;
 }
}

• radarbob: I don't see a class anywhere; I suspect this will not compile.
• Unlived172: I copied the code straight from Visual Studio Code, so the using, class, and Main lines are hidden sorry, I haven't thrown it in Visual Studio to commit the project to a finished project yet.
• radarbob: Is this the entire program? only one method? only part of one method? Include its scope - i.e. the method name-signature with the closing curly brace; and the class "block" if is relevant to the code under review. Very importantly, if the code as shown does not compile or not work then it is not possible to review.
• Hayden: @radarbob This will compile as it's using top-level statements.

Answer 1

First of all, congratulations on completing the project! A working program that does what you want is an important first step. There's a lot here that you can build on and the organization is decent.

As a general note, I'll say that anytime you are writing similar code over and over, you should wonder if there is a better approach. In particular, GameWon is ripe for cleaning up. One way to draw inspiration is to think how you would handle a variable board size. The way you wrote it, moving to a 4x4 board requires totally rewriting this function. The central checking could be more like:

// check for a win on the diagonal
bool xWinsDiag = true;
for (int i=0; i < BoardSize; ++i)
{
 xWinsDiag = xWinsDiag && board[i, i] == "X";
}
if (xWinsDiag)
 return true;
// check for a horizontal win
for (int row = 0; row < BoardSize; ++row)
{
 bool xWinsRow = true;
 for (int col = 0; col < BoardSize; ++col)
 {
 xWinsRow = xWinsRow && board[col, row] == "X";
 }
 if (xWinsRow)
 return true;
}
// don't forget to check the anti-diagonal and columns

Another thought for this function is that it could be called GetGameState return a type like:

enum GameState {Active, XWon, YWon, Draw}

Now, I have X hard-coded in here and you also need to check for O, which you can do with a loop over the player. Or you could realize that in each case (diagonal, row, column) the symbol in the first spot is the only player than can win that row, so you only need to check that the others match.

Similarly, CPUmove is a lot of duplicated code, can you think of a simpler way to write it? The same code repeated suggests a loop; what you are doing is testing the squares in a particular order and taking the first open one. If you had that order stored in an array, you could simply loop over them.

Your main game loop is two very similar blocks of code. In the first case, the loop goes:

1. CPUmove
2. DisplayGameBoard
3. GetSelection
4. DisplayGameBoard
5. CPUmove
6. DisplayGameBoard
7. GetSelection
8. DisplayGameBoard
9. etc

And in the second case, the loop is:

1. DisplayGameBoard
2. GetSelection
3. DisplayGameBoard
4. CPUmove
5. DisplayGameBoard
6. GetSelection
7. DisplayGameBoard
8. CPUmove
9. etc

Hopefully you see how similar those are: the only difference with the first list is that the CPU moves first. So you can have that conditionally occur, and then enter a common loop. And that makes sense, when it's the players first turn, either the board will be empty or it will contain a single O, but other than that the game logic is identical.

The other area that could use cleaning up is handling user input. It looks like it'll work correctly if the user enters the values exactly like they are supposed to, but if they don't any number of bad things will happen. You also don't check if the square is empty, which means X can steal O's squares! Your GetSelection needs to run in a loop until the user enters a value that matches the expected format (a-c, 0-2) that names an empty square. If the input is invalid, the program needs to say why. Writing this loop might make you realize it's not always possible to exit - what if the board is full i.e. the game is a tie? (You might have also discovered this case writing your main loop - is a win the only way for a game to end?) I know the validation feels unnecessary because you know what your program expects, but it's a good habit to get into and is crucial if anyone else is going to use this program.

You also have a few global variables like spaceSelection that you are using for processing input. I would get rid of those and have the functions return values that you store in a local variable. Code is much easier to read if it's clear how the data flows, and global variables obscure that.

As for organization, this is a simple enough program that it's ok to put it all in one file. If you wanted to pull out classes, the Board class is pretty obvious. It would maintain state and could look like

class Board
{
 void MakeMove(int c, int r, char player);
 GameState GetGameState();
 void PrintBoard();
}

I think a CPU class makes sense, and you could have different implementations to adjust the difficulty level.

interface ICpu
{
 void PlayNextMove(Board b);
}
class EasyCpu : ICpu {}
class HardCpu : ICpu {}
class ExpertCpu : ICpu {}

It would be nice to put all of the input parsing and validating in one place, maybe

class UserInput
{
 Tuple<int, int> ReadNextMove();
}

It feels like I've written a lot, but really it's because your code is very close to being quite good. I think you're just a few insights away from being able to improve it a lot.

• radarbob: if (xWinsRow) return true; => This is the last line of the method, I assume. I suspect void is returned if that statement is false. In that case I expect a runtime error if the method signature says it is returning a boolean.
• Unlived172: Wow thanks a ton man! I will definitely keep those improvements in mind when I make my next project. I really appreciate the full explanation actually, helps me commit it to memory. I'm trying to get out of that tutorial nightmare and actually make stuff. I was tempted to make a CPU class and a Player class but I didn't see anything interesting that could be done with that method. Would that have been a better option? Again thanks for reading over my stuff and taking the time to leave such a nice explanation!
• Pierre Menard: @radarbob you're right, I should make it clearer that this isn't meant to be the full body of the function