Connect 4 (glorified tic-tac-toe) implementation

I'm posting here my implementation of the game Connect 4. It's obviously a pretty trivial game and code but I'm primarily interested in any advice concerning improvements to the code structure: types, dependencies and coupling, concerns, etc.

I'm interested in attacking more complicated games but I think it's best to start with really simple cases, learn all the best practices and slowly build upon that. I think this game is as good as any to start learning some trivial (for the moment) game implementation.

My code is structured in the following main classes (sorry it's in Spanish):

• AIEngine: Computer player AI

• Game: Basic game mechanics

• Player: Human and computer players

• Board: Game board

• Judge: Rule enforcer

There are some additional auxiliary classes and enums:

```
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;

namespace Conect4
{
internal sealed class AIEngine
{
readonly int maximumDepth;
readonly Random random;

public AIEngine(DifficultyLevel difficultyLevel)
{
this.maximumDepth = (int)difficultyLevel;

if (maximumDepth (int)DifficultyLevel.Hard)
throw new ArgumentOutOfRangeException("difficultyLevel");

this.random = new Random(DateTime.Now.Millisecond);
}

public int GetBestMove(Board board, ActivePlayer player)
{
if (board == null)
throw new ArgumentNullException("board");

var node = new Node(board);
var possibleMoves = getPossibleMoves(node);
var scores = new double[possibleMoves.Count];
Board updatedBoard;

for (int i = 0; i ();

for (int i = 0; i maximumScore)
{
goodMoves.Clear();
goodMoves.Add(i);
maximumScore = scores[i];
}
else if (scores[i] == maximumScor

Many of the loops can be replaced with LINQ to increase readability and safety. Also, method names, even private ones, should be UpperCamelCase. Let's first look at the getPossibleMoves method:

private List getPossibleMoves(Node node)
{
    var moves = new List();

    for (int i = 0; i < Board.BoardColumns; i++)
    {
        if (node.Board.GetCellState(0, i) == CellStates.Empty)
        {
            moves.Add(i);
        }
    }

    return moves;
}

The method body can be made into a one-liner using LINQ:

private List getPossibleMoves(Node node)
{
    return Enumerable.Range(0, Board.BoardColumns)
                     .Where(x => node.Board.GetCellState(0, x) == CellStates.Empty)
                     .ToList();
}

The following at the end of the scoreNode method:

foreach (var varianteContrincante in nodo.Variants)
{
    score += scoreNode(varianteContrincante, player, depth + 1);
}

can be replaced with the following LINQ:

score += nodo.Variants.Sum(x => scoreNode(x, player, depth + 1));

Let's look at the CheckForVictory method:

public static bool CheckForVictory(ActivePlayer player, Board tablero)
{
    if (tablero == null)
        throw new ArgumentNullException("board");

    for (int i = 0; i < Board.BoardRows; i++)
    {
        for (int j = 0; j < Board.BoardColumns; j++)
        {
            if (tablero.GetCellState(i, j) == (CellStates)player)
            {
                if (checkForVictory(tablero, i, j))
                    return true;
            }
        }
    }

    return false;
}

The for loops can be replaced using LINQ:

public static bool CheckForVictory(ActivePlayer player, Board tablero)
{
    if (tablero == null)
        throw new ArgumentNullException("board");

    return
       (from x in Enumerable.Range(0, Board.BoardRows)
        from y in Enumerable.Range(0, Board.BoardColumns)
        where tablero.GetCellState(x, y) == (CellStates)player &&
              checkForVictory(tablero, x, y)
        select true).Any();
}

The checkForVictory method has a lot of repetition in it. Here's my try at reducing the repetition. Some might consider my version ugly. Go with what is more readable to you.

private static bool checkForVictory(Board board, int fila, int columna)
{
    bool searchRight = columna = requiredCellsInARow - 1;
    bool searchUp = fila > Board.BoardRows - requiredCellsInARow;
    bool searchDown = fila  columna - x).ToList();
    var ysDown = Enumerable.Range(fila, 4).ToList();
    var ysUp = Enumerable.Range(0, 4).Select(x => fila - x).ToList();

    return
        (searchRight && checkCells(xsRight.Select(x => board.GetCellState(fila, x)).ToArray())) ||
        (searchLeft && checkCells(xsLeft.Select(x => board.GetCellState(fila, x)).ToArray())) ||
        (searchUp && checkCells(ysUp.Select(y => board.GetCellState(y, columna)).ToArray())) ||
        (searchDown && checkCells(ysDown.Select(y => board.GetCellState(y, columna)).ToArray())) ||
        (searchLeft && searchUp && CheckCells(board, ysUp, xsLeft)) ||
        (searchLeft && searchDown && CheckCells(board, ysDown, xsLeft)) ||
        (searchRight && searchUp && CheckCells(board, ysUp, xsRight)) ||
        (searchRight && searchDown && CheckCells(board, ysDown, xsRight));
}

with the helper method:

private static bool CheckCells(Board board, IEnumerable rows, IEnumerable columns)
{
    return checkCells(rows.Zip(columns, (y, x) => board.GetCellState(y, x)).ToArray());
}

The checkCells method can be rewritten using LINQ.

private static bool checkCells(params CellStates[] celdas)
{
    Debug.Assert(celdas.Length == requiredCellsInARow);

    for (int i = 1; i < requiredCellsInARow; i++)
    {
        if (celdas[i] != celdas[0])
            return false;
    }

    return true;
}

Here is the LINQ version:

private static bool checkCells(params CellStates[] celdas)
{
    Debug.Assert(celdas.Length == requiredCellsInARow);

    return celdas.Skip(1).All(x => x == celdas[0]);
}

On to the changeActivePlayer method:

private void changeActivePlayer()
{
    if (activePlayer == humanPlayer)
    {
        activePlayer = computerPlayer;
    }
    else
    {
        activePlayer = humanPlayer;
    }
}

I prefer to use expressions instead of statements when possible. In this case we can replace the if statement with an expression using the conditional operator:

private void changeActivePlayer()
{
    activeplayer = activePlayer == humanPlayer ? computerPlayer : humanPlayer;
}

You can also change the following code in the ConsoleGame constructor:

if (computerPlaysFirst)
{
    activePlayer = computerPlayer;
}
else
{
    activePlayer = humanPlayer;
}

with:

activePlayer = computerPlaysFirst ? computerPlayer : humanPlayer;

StackExchange Code Review Q#96545, answer score: 4