7
\$\begingroup\$

A Minesweeper library in C#. To be included with any GUI stack.

Did some breakpoint tests and unit testing, seems to work fine.

I'm asking if the code makes sense to everyone, and if there can be improvements.

Three classes: Tile, Board and Game.

Tile

public enum TileStatus
{
    Revealed,
    Flagged,
    Hidden
}

public class Tile
{
    public Tile(int x, int y)
    {
        X = x;
        Y = y;
    }

    public int X { get; }
    public int Y { get; }

    public int AdjacentMines { get; internal set; }
    public bool HasAdjacentMines => AdjacentMines != 0;

    public bool IsMine { get; internal set; }

    public TileStatus Status { get; private set; } = TileStatus.Hidden;

    public bool IsRevealed => Status == TileStatus.Revealed;
    public bool IsFlagged => Status == TileStatus.Flagged;

    internal void SetRevealed() => Status = TileStatus.Revealed;
    internal void SetFlagged() => Status = TileStatus.Flagged;
    internal void SetHidden() => Status = TileStatus.Hidden;
}

Board

public class Board
{
    private Tile[,] _tiles;

    public int Columns => _tiles.GetLength(0);
    public int Rows => _tiles.GetLength(1);

    public Board(int columns, int rows)
    {
        _tiles = new Tile[columns, rows];
        InitializeTiles(columns, rows);
    }

    private void InitializeTiles(int columns, int rows)
    {
        for (int x = 0; x < columns; x++)
            for (int y = 0; y < rows; y++)
                _tiles[x, y] = new Tile(x, y);
    }

    public Tile this[int x, int y] => _tiles[x, y];
}

Game

public interface IGame
{
    Board Board { get; }
    int Columns { get; }
    int Rows { get; }
    int Mines { get; }

    IList<Tile> Reveal(int x, int y);

    void Flag(int x, int y);
    void Unflag(int x, int y);

    string VisualizeCurrentState();
    string VisualizeEverything();
}

public class Game : IGame
{
    public Game(int columns, int rows, int mines)
    {
        if (columns < 1 || rows < 1)
            throw new ArgumentException("Board can't be smaller than 1x1.");

        if (mines >= columns * rows)
            throw new ArgumentException("Must be fewer mines than tiles.");

        Mines = mines;
        Board = new Board(columns, rows);
    }

    private bool _isFirstReveal = true;

    public Board Board { get; }
    public int Columns => Board.Columns;
    public int Rows => Board.Rows;
    public int Mines { get; }

    /// <summary>
    /// Reveal tiles and return list of revealed tiles.
    /// 
    /// If the initial tile was blank (no mines, no adjacent mines), reveals and returns a list of safe tiles.
    /// If the initial tile was a mine (game over), reveals and returns a list of all mines.
    /// </summary>
    public IList<Tile> Reveal(int initialX, int initialY)
    {
        if (initialX >= Columns || initialX < 0 || initialY >= Rows || initialY < 0)
            throw new ArgumentException("Given (x, y) out of bounds.");

        // Populate the board with mines after first click, because first click must be safe.

        if (_isFirstReveal)
        {
            _isFirstReveal = false;
            PopulateTiles(initialX, initialY);
        }

        // Set the revealed tile's status.

        var initialTile = Board[initialX, initialY];
        initialTile.SetRevealed();

        // Prepare the return list.

        var area = new HashSet<Tile>
        {
            initialTile
        };

        if (initialTile.IsMine)
        {
            // If the clicked tile was a mine, reveal all mines and return them.

            area.UnionWith(RevealMines());
        }
        else if (!initialTile.HasAdjacentMines)
        {
            // If the clicked tile was blank (no mine, no adjacent mines), reveal a surrounding safe area.

            SearchRecursively(initialTile);
        }

        return area.ToList();

        // Method for finding safe area around a tile.

        void SearchRecursively(Tile fromTile)
        {
            InvokeOnAdjacentTiles(fromTile, (tile) =>
            {
                if (tile.IsMine || tile.IsFlagged || tile.IsRevealed)
                    return;

                tile.SetRevealed();

                area.Add(tile);

                if (!tile.HasAdjacentMines)
                    SearchRecursively(tile);
            });
        }
    }

    /// <summary>
    /// Reveals all mines and return a list.
    /// </summary>
    private IList<Tile> RevealMines()
    {
        var mines = new HashSet<Tile>();
        for (int x = 0; x < Columns; x++)
        {
            for (int y = 0; y < Rows; y++)
            {
                var tile = Board[x, y];

                if (tile.IsMine)
                {
                    tile.SetRevealed();
                    mines.Add(tile);
                }
            }
        }
        return mines.ToList();
    }

    /// <summary>
    /// Populate all tiles with mines and numbers. The initial tile should not have a mine.
    /// </summary>
    private void PopulateTiles(int initialX, int initialY)
    {
        // Populate mines.

        var random = new Random();
        var placed = 0;

        while (placed < Mines)
        {
            var x = random.Next(Columns);
            var y = random.Next(Rows);

            if (!(x == initialX && y == initialY) && !Board[x, y].IsMine)
            {
                Board[x, y].IsMine = true;
                placed++;
            }
        }

        // Populate tiles with adjacent mines count.

        for (int x = 0; x < Rows; x++)
        {
            for (int y = 0; y < Columns; y++)
            {
                var tile = Board[x, y];
                if (tile.IsMine)
                    continue;
                tile.AdjacentMines = CountAdjacentMines(tile);
            }
        }

        int CountAdjacentMines(Tile tile)
        {
            var count = 0;
            InvokeOnAdjacentTiles(tile, (a) => {
                if (a.IsMine)
                    count++;
            });
            return count;
        }
    }

    /// <summary>
    /// Invoke an action on adjacent tiles.
    /// </summary>
    private void InvokeOnAdjacentTiles(Tile tile, Action<Tile> action)
    {
        for (int nx = tile.X - 1; nx <= tile.X + 1; nx++)
        {
            if (nx < 0 || nx == Columns)
                continue;

            for (int ny = tile.Y - 1; ny <= tile.Y + 1; ny++)
            {
                if (ny < 0 || ny == Rows)
                    continue;

                if (nx == tile.X && ny == tile.Y)
                    continue;

                action.Invoke(Board[nx, ny]);
            }
        }
    }

    /// <summary>
    /// Flag a tile.
    /// </summary>
    public void Flag(int x, int y)
    {
        if (x >= Columns || x < 0 || y >= Rows || y < 0)
            throw new ArgumentException("Given (x, y) out of bounds.");

        Board[x, y].SetFlagged();
    }

    /// <summary>
    /// Unflag the tile.
    /// </summary>
    public void Unflag(int x, int y)
    {
        if (x >= Columns || x < 0 || y >= Rows || y < 0)
            throw new ArgumentException("Given (x, y) out of bounds.");

        Board[x, y].SetHidden();
    }

    /// <summary>
    /// Get a text visualization of the current state of the board.
    /// </summary>
    public string VisualizeCurrentState()
    {
        var sb = new StringBuilder();

        for (var y = 0; y < Rows; y++)
        {
            for (var x = 0; x < Columns; x++)
            {
                var tile = Board[x, y];

                if (!tile.IsRevealed)
                    sb.Append(" ");
                else if (tile.IsFlagged)
                    sb.Append(">");
                else if (tile.IsMine)
                    sb.Append("x");
                else if (!tile.HasAdjacentMines)
                    sb.Append("_");
                else
                    sb.Append(tile.AdjacentMines.ToString());

                if (x == Columns - 1)
                    sb.Append("\n");
            }
        }

        return sb.ToString();
    }

    /// <summary>
    /// Get a text visualization of everything on the board, including hidden tiles.
    /// </summary>
    public string VisualizeEverything()
    {
        var sb = new StringBuilder();

        for (var y = 0; y < Rows; y++)
        {
            for (var x = 0; x < Columns; x++)
            {
                var tile = Board[x, y];

                if (tile.IsMine)
                    sb.Append("x");
                else if (!tile.HasAdjacentMines)
                    sb.Append("_");
                else
                    sb.Append(tile.AdjacentMines.ToString());

                if (x == Columns - 1)
                    sb.Append("\n");
            }
        }

        return sb.ToString();
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Just a comment on the naming of GetAdjacentTiles: It doesn't get tiles, it applies an action on tiles. I would name it ApplyOnAdjacentTiles. \$\endgroup\$ – Olivier Jacot-Descombes Aug 5 '18 at 17:01
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Feel free to write a follow-up question, including your unit tests, instead. \$\endgroup\$ – Mast Aug 6 '18 at 11:53
10
\$\begingroup\$

Looks alright to me. Some minor issues:

1) I don't like those methods:

internal void SetRevealed() => Status = TileStatus.Revealed;
internal void SetFlagged() => Status = TileStatus.Flagged;
internal void SetHidden() => Status = TileStatus.Hidden;

IMHO, they bloat your class and hardly bring anything to the table. I would prefer writing Status = TileStatus.Revealed explicitly. But a good middle ground is to refactor those methods into extensions. It would keep the class clean, but it still would allow you to use those "shortcuts". The same can be said about IsXXXXX properties.

2) For similar reasons I don't like IGame.Rows and IGame.Columns. Those are the properties of the game board, not of the game itself. So it makes sense to me that they should be exposed and accessed via Board property.

3) IGame lacks the property that describes the state of the game. Has player won? Has s/he lost? Is game still in progress? No easy way to tell. You might also want to count the number of moves that player has made.

4) VisualizeCurrentState() and VisualizeEverything() - I feel like those methods violate SRP and should be moved to different class. Visualization logic has nothing to do with game logic after all.

5) Flag and Unflag methods should probably return a Tile.

6) I don't like this portion:

    if (_isFirstReveal)
    {
        _isFirstReveal = false;
        PopulateTiles(initialX, initialY);
    }

Why can't you have IGame.Restart() or IGame.Init() method and do the initialization there?

\$\endgroup\$
  • \$\begingroup\$ Thanks for the response. I’m going to add Init() method or similar. But still must place mines after the first click. According to Minesweeper rules, the first click must be safe. \$\endgroup\$ – Jam Aug 7 '18 at 7:22
  • \$\begingroup\$ @Jam good point, I've missed your comment about that. You can still populate the board beforehand, and simply move the mine to another tile, if player hits it on his first move, but I am not sure that this approach is better, than what you have. \$\endgroup\$ – Nikita B Aug 8 '18 at 9:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.