Evaluation function for minimax/alphabeta algorithm for Tic Tac Toe AI

I am trying to develop an optimal evaluation function to use in minimax/alpha-beta algorithm for developing tic-tac-toe AI.

I am counting number of circles/crosses in a row/column/diagonal with empty space behind it (with three-in-a-row, there is no empty space). Based on number of symbols in such line, I multiply the separate scores with \$10^{\text{counter} - 1}\$, which results in \$1,10 \text{ or } 100\$ points.

I am sure much can be improved, because optimal solution is rarely found and I am having problems using this function in alphabeta algorithm

My question is - How can this function be improved? Small pieces of code and suggestions appreciated.

My code:

```
private int h(int[][] field, int depth, int player) //final score of the node
{
if (win(field, 1)) //if human won
return -1000; //very bad for MAX=computer
if (win(field, 0)) //if computer won
return 1000;

int heuristics = individualScore(field, 0) - individualScore(field, 1);
return heuristics;
}

private int individualScore(int[][] field, int player)
{

int sum = 0;
int otherPlayer = -1;
if (player == 0) //if computer is the current player
otherPlayer = 1; //other player is human
else
otherPlayer = 0;//Vice versa
for (int i = 0; i 0)
sum += (int)Math.Pow(10, counter - 1);
}

for (int i = 0; i 0)
sum += (int)Math.Pow(10, counter - 1);
}
int counterD = 0;
bool diagonalAvailable = true;
for (int i = 0; i 0)
sum += (int)Math.Pow(10, counterD - 1);
counterD = 0;
diagonalAvailable = true;
int j = 0;
for (int i = 2; i >= 0; i--)
{
if (field[i][j] == player)
counterD++;

I'm going to sidestep the complexities of machine learning itself, it is after all a code review.

Unclear method names and unused parameters

private int h(int[][] field, int depth, int player) //final score of the node

h is a really bad method name. What doest this method do? h isn't particularly explanatory. calculateHeuristics would be considerably better.

I have a lot to say on int[][] field which I will address at a later stage in detail.

Both depth and player are part of the method parameters, but they are never used. You should remove them from the parameter list.

Code readability

if (win(field, 1)) //if human won

if (win(field, 0)) //if computer won

0 and 1 are not particularly readably values for "human" and "computer". Use an enum instead. The good thing is that enums can automatically translate to an int, which means that the odds of encountering breaking changes is minimized.

public enum Player
{
    Computer = 0,
    Human = 1
}

And then your code becomes much easier to read, you wouldn't even need the comments anymore:

if (win(field, Player.Human))

if (win(field, Player.Computer))

Secondly, I would rename field to board. "Field" makes it ambiguous as to whether you're referring to the board (similar to "the football field") or a cell on the board (similar to "a form field").

Thirdly, win is not a good name for a method. I suggest renaming it to IsVictoryFor (or any similarly descriptive name). If you then also create a custom class for your board (instead of using int[][] - I will elaborate on this later in the answer), you can move that method to that custom class. This would give you a massive boost in readability:

if(board.IsVictoryFor(Player.Human))

Encapsulation and SRP

int[][] field

I strongly suggest wrapping your int[][] in a custom class. This allows to you use class methods, which help you categorize your methods.

In your current code, you severely lack any application of Single Responsibility Principle (SRP). Especially in topics such as machine learning, you'll find that the complexity of the code increases exponentially, and you should be prepared for this by properly separating your responsibilities before things get out of hand.

As the adage goes, failure to plan is planning to fail. To that end, I suggest improving your code to make future changes/extensions as seamless as possible.

Don't use ints as placeholder values

You have on several occasions used ints to store your data (players, tokens on board cells). Every time you do that, you inherently require whoever reads your code to be aware of the mapping between the used int value and its particular meaning. This is hugely detracting from your code readability.

As a simple fix, you should use enums.

public enum Player { Computer = 0, Human = 1 }
public enum CellValue { Empty = 0, X = 1, O = 2 }

This dramatically increased the readability of your code, as you no longer will need to use hardcoded magical numbers (0 and 1) and can instead rely on human-readable values (Token.X, Player.Human, ...)

Note: You are cleverly relying on the player int matching with the cell int. I would advise against this. If you equate the two, you're effectively hardcoding which player goes first, and it'll be nigh impossible to reuse your code to have the players switch places (or when you want the compluter to play against itself, which you are inevitably going to have to do if you want to run bulk simulations).

Separating the two does come with the disadvantage that you need to perform some additional checking logic, but the reusability of the code makes it worth it.

Magic numbers

Other than the values I already suggested to encapsulate in enums, you also use other literal values throughout your code:

return -1000;

return 1000;

While you luckily didn't spread this throughout your codebase, the principle still applies; you should use const values.

public readonly const int HEURISTIC_POINTS_VICTORY = 1000;
public readonly const int HEURISTIC_POINTS_LOSS = -1000;

This also makes it a lot easier if you want to tweak the numbers in the future, because you only have to adjust them in one location.

Reusability

Currently, you're using powers of 10 to calculate the score (1,10,100). You have used this calculation in multiple locations:

sum += (int)Math.Pow(10, counter - 1);

sum += (int)Math.Pow(10, counter - 1);

sum += (int)Math.Pow(10, counterD - 1);

sum += (int)Math.Pow(10, counterD - 1);

This is not good. If you decide to change your scoring algorithm tomorrow, you're liable to only update some of the code and potentially forget other instances of the same calculation logic. This is a breeding ground for bugs and unexpected behavior.

You can abstract this to a separate method:

public int CalculateScore(int numberOfTokensInLine)
{
    return Math.Pow(10, numberOfTokensInLine);
}

Shou

StackExchange Code Review Q#158350, answer score: 5