# Checking if naughts or crosses win in Tic Tac Toe

I am working on a coding challenge for Tic Tac Toe. I am calculating the results and inserting them into the database.

However I have a long and messy if statement, which checks if the array's values match a valid win for O or X. I feel like it could be refactored down but I'm unsure on how to approach this.

    /** @var int */
protected $xWins = 0; /** @var int */ protected$oWins = 0;

/** @var int */
protected $draws = 0; /** */ public function __construct() { // creates a new database on object creation$this->database = new Database();
}

public function winner(string $input) {$input = str_replace("\\n", "", $input);$length = strlen($input); for ($i = 1; $i <=$length; $i++) { if ($i % 9 === 0) {

$result = substr($input, $i - 9, 9); if ($result[0] === "X" && $result[1] === "X" &&$result[2] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[3] === "X" && $result[4] === "X" &&$result[5] === "X") {
$winner = "x";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[6] === "X" && $result[7] === "X" &&$result[8] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[0] === "X" && $result[3] === "X" &&$result[6] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[1] === "X" && $result[4] === "X" &&$result[7] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[2] === "X" && $result[5] === "X" &&$result[8] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[0] === "X" && $result[4] === "X" &&$result[8] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } else if ($result[2] === "X" && $result[4] === "X" &&$result[6] === "X") {
$winner = "X";$this->database->insert($winner,$result);
$this->xWins++; } // checks for O lines else if ($result[0] === "O" && $result[1] === "O" &&$result[2] === "O") {
$winner = "o";$this->database->insert($winner,$result);
$this->oWins++;; } else if ($result[3] === "O" && $result[4] === "O" &&$result[5] === "O") {
$winner = "o";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[6] === "O" && $result[7] === "O" &&$result[8] === "O") {
$winner = "o";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[0] === "O" && $result[3] === "O" &&$result[6] === "O") {
$winner = "o";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[1] === "O" && $result[4] === "O" &&$result[7] === "O") {
$winner = "O";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[2] === "O" && $result[5] === "O" &&$result[8] === "O") {
$winner = "O";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[0] === "O" && $result[4] === "O" &&$result[8] === "O") {
$winner = "O";$this->database->insert($winner,$result);
$this->oWins++; } else if ($result[2] === "O" && $result[4] === "O" &&$result[6] === "O") {
$winner = "O";$this->database->insert($winner,$result);
$this->oWins++; // checks for draw } else {$winner = "Draw";
$this->database->insert($winner, $result);$this->draws++;
}
}
}

• May we see a realistic sample input (a couple of games passed to this method)? I think I can reverse engineer one from your code logic, but it would be better if I could have my assumptions confirmed. Why aren't x's and o's saved to the database consistently (regarding upper/lowercase)? – mickmackusa Jun 6 '20 at 22:41
• Are unused cells of the gameboard filled with a visible character or are spaces used? I am assuming the game short-circuits to a winner as soon as a winner is determined. – mickmackusa Jun 6 '20 at 22:50
• In your next questions, please post the complete code of a file. This code is missing the class declaration, which makes it harder than necessary for potential reviewers to run your code. – Roland Illig Jun 7 '20 at 8:31

This reminds me of a Battleship gameboard parser that I once designed. https://stackoverflow.com/a/47595575/2943403 a rare case where regex outperforms non-regex techniques.

I am going to recommend a class variable that is an associative array so that variable variables can be avoided. By filling each subarray with the respective $result strings, you compress your required data into a single structure instead of having a collection of results and a win/draw incrementer. protected$outcomes = [
'X' => [],
'O' => [],
'D' => []
];


Your str_replace(), strlen(), and subsequent modulus-based looping is definitely working too hard. It will be far simpler to break the string into substrings by using:

foreach (explode('\n', $input) as$result) {


Now that game results are properly separated, you can use a regular expression to concisely parse the string and return the outcome as a single letter.

A "branch reset" ((?|...)) is used so that the matching character is always \1 throughout each branch versus \1, \2, \3, \4. The x pattern flag allows me to use spaces and comments to separate and explain pattern logic.

public assessGames($input) { foreach (explode('\n',$input) as $result) {$outcome = preg_match(
'~(?|
^(?:.{3}){0,2}(\S)\1{2}   #horizontal win
|(\S)(?:.{2}\1){2}        #or vertical win
|^(\S)(?:.{3}\1){2}       #or diagonalLTR win
|^.{2}(\S)(?:.\1){2}      #or diagonalRTL win
)~x',
$result,$match
)
? $match[1] // X or O : 'D';$this->outcomes[$outcome][] =$result;
}
}


Note, if you are parsing a high volume of games, you may like to refactor this approach to skip the explode&iterate step and make a single preg_match_all() call.

Now that your outcomes are fully evaluated, then saved to the class variable, make as few trips to the database as possible. I always discourage making looped trips to the db. I don't know what utilities you have in your database class, but using a batch insert technique will be beneficial here.

If/When you need to know how many wins/losses/draws a player has, just count the elements in the class variable.

Granted, the four-part pattern that I have designed may seem like incomprehensible magic to someone who is not familiar with the syntax, but by studying the pattern explanation at regex101.com, you can learn the full meaning. I have labeled each pattern to aid in understanding what each is doing.

Be sure to break your class functionality into separate methods so that each method has a "single responsibility".

• You have "hardcoded" variable names like $xWins and $oWins. For my answer to cooperate with your variable naming convention, I would need to take my $match[1] and use dynamic variable syntax. stackoverflow.com/questions/9257505/… This (anti-pattern) is almost always a symptom of bad code design. To prevent this, I am naming three keys in an array to store the outcomes. This is a clean and logic design because the values are sensibly related. – mickmackusa Jun 11 '20 at 22:37 • You can replace my "you" for "the OP" then. I am trying to explain. What is still foggy? I don't want to keep the OP's indidualized variables because then my preg_ result would need to be like ++${ strtolower($match[1]) . 'Wins'}; and that is a mess. I am recommending a wholly refined storage technique and a change to what exactly is being stored in the class level variable. – mickmackusa Jun 11 '20 at 23:28 • No, I am not calling them the same thing. Variable variables are the problem. An associative array is the solution/replacement. I don't recommend the use of $xWins and $oWins -- I am recommending an associative array to eliminate those variables. protected$outcomes – mickmackusa Jun 11 '20 at 23:55
• No bother, maybe I didn't explain myself very well. – mickmackusa Jun 12 '20 at 0:03

I wouldn't call myself an expert, but I finished a tic-tac-toe project today as well, I was writing Python, so my suggestions might not be idiomatic for PHP - but I think this will apply to your situation as well.

Your intuition is solid. Every time you find yourself using cut and paste, this is probably a sign you can put the code inside a function.

Idea One:

For example, create a function inside winner - this contains the valid win matching mode. this function takes a parameter "x_or_o" - which is either a string 'x' or a string 'o'.

This should be a great start because it will cut your checking code in half. Let's see if we can do better.

Idea Two:

I notice that your code doesn't care about whether a player has a winning row, or a winning column. In this case, you can assign your logical operators to a variable. e.g.

public function win_check(string $x_or_o) { top_row_win =$result[1] === x_or_o && $result[4] === x_or_o &&$result[7] === x_or_o
mid_row_win = $result[2] === x_or_o &&$result[5] === x_or_o && $result[8] === x_or_o ... win = (top_row_win || mid_row_win || bottom_row_win || left_col_win || mid_col_win || right_col_win || diagonal_win || off_diagonal_win) if (win) {$winner = x_or_o;
$this->database->insert($winner, \$result)
if (x_or_o == 'x'){
xWins++
}
elif (x_or_o == 'o') {
oWins++
}
}
}

//Then in your outer scope, your win_check function will be called from winner like this.

win_check(x)
win_check(o)


Idea Three:

In my project, I used a 2d array and used index slicing to do this more elegantly. I do not know what array slicing or index slicing is possible in PHP.