3
\$\begingroup\$

I've created a simple Snakes and Ladders game. The goal is improving my knowledge about OOP.There are some basic rules.

  1. I need to be able to move my token.
  2. The token starts in the square 1.
  3. If the token is on square 1 and is moved by 3 then the token is on square 4.
  4. A player can win the game.
  5. The first who reaches the square 100 is the winner.
  6. You can't over-square the board. If you do it your position will remain the same. (97 + 4 = 97)
  7. The moves are based on a roll dice between 1 and 6.
  8. The player rolls the dice, then moves the token.

Keeping all this in mind as I did.

Class CBoard.cs

using System;
using System.Collections.Generic;

namespace Snakes_and_ladders
{
    class CBoard
    {
        private int[] board;
        List<CPlayer> players = new List<CPlayer>();

        public int[] Board { get => board; }
        public CBoard()
        {
            // A 100-cell board is created by default.
            board = new int[100];
            Array.Clear(board, 0, board.Length);
        }

        // Function to create Ladders and Snakes.Se changes the value
        // of the array in the index [i-1] being i the key of the
        // dictionary.the value that is saved in that index corresponds
        // to the value of the board where the player moves in case of
        // falling into said Index.
        //
        // Ex: Key => 2, Value => 10 implies that there is a ladder that
        // goes from cell 1 to cell 9 of the board.
        private void createSnakesOrLadders(Dictionary<int, int> dataDict)
        {
            foreach (KeyValuePair<int, int> data in dataDict)
            {
                board[data.Key - 1] = data.Value - 1;
            }
        }

        // Default constructor overload
        // Creates an A x L board by adding ladders and snakes.
        public CBoard(int altura, int largo,
            Dictionary<int, int> ladders = null, Dictionary<int, int> snakes = null)
        {

            // At a minimum, a 2x2 board is necessary.
            if (altura < 2 || largo < 2)
                throw new Exception("The height and length need to be at least greater than 1.");

            // Initial size of the number of ladders and snakes on the board.
            int ladderSize = 0;
            int snakesSize = 0;

            // If the board is not null, we save the actual number of ladders and snakes.
            if (!(ladders is null))
                ladderSize = ladders.Count;

            if (!(snakes is null))
                snakesSize = snakes.Count;

            // We create the board, with values set to 0.
            board = new int[altura * largo];
            Array.Clear(board, 0, board.Length);

            // If the total size of the number of ladders and snakes is less than half the board
            // ladders and snakes are created on the board. If not, the exception is thrown.
            if ((ladderSize * 2) + (snakesSize * 2) / 2 < board.Length)
            {
                if (!(ladders is null))
                    createSnakesOrLadders(ladders);

                if (!(snakes is null))
                    createSnakesOrLadders(snakes);
            }
            else
            {
                throw new Exception("The total sum of Snakes and Ladders cannot exceed 50% of the board.");
            }
        }
    }
}

Class CPlayer.cs

using System;
using System.Threading;

namespace Snakes_and_ladders
{
    class CPlayer
    {
        int[] board;
        private int position = 0;
        private string nickName = null;
        private int diceResult = 0;
        private bool winner = false;

        public int Position { get => position + 1; }
        public int DiceResult { get => diceResult;  }
        public string NickName { get => nickName; }
        public bool Winner { get => winner; }


        public CPlayer(string nickName, CBoard board) {
            this.nickName = nickName;
            this.board = board.Board;
        }

        public void Roll()
        {
            // Wait 30 milliseconds to change the random seed.
            Random rnd = new Random();
            Thread.Sleep(30);
            diceResult = rnd.Next(1, 7);
        }

        public void Move()
        {
            // Move the player N dice cells.
            if (position + diceResult < board.Length)
                if (board[position + diceResult] == 0)
                    position = position + diceResult;
                else
                    position = board[diceResult + position];

            if (position == board.Length - 1)
                winner = true;
        }
    }
}

Program.cs

using System;
using System.Collections.Generic;

namespace Snakes_and_ladders
{
    class Program
    {
        static void Main(string[] args)
        {
            // Ladders
            Dictionary<int, int> ladderDicctionary = new Dictionary<int, int>()
            {
                {2, 38}, {7, 14}, {8, 31}, {16, 26}, {21, 42},
                {28, 84}, {36, 44}, {51, 67}, {71, 91}, {78, 98}, {87, 94}
            };

            // Snakes
            Dictionary<int, int> snakesDicctionary = new Dictionary<int, int>()
            {
                {15, 5}, {48, 10}, {45, 24}, {61, 18}, {63, 59},
                {73, 52}, {88, 67}, {91, 87}, {94, 74}, {98, 79}
            };

            CBoard customBoard = new CBoard(10, 10, ladderDicctionary, snakesDicctionary);

            // Board by default.
            CBoard board = new CBoard();

            List<CPlayer> players = new List<CPlayer>();

            int n_players = 0;
            do
            {
                Console.Write("Enter the number of players: ");
                n_players = Convert.ToInt32(Console.ReadLine());

                if(n_players <= 1)
                    Console.WriteLine("The total of players need to be 2 or more.");

            } while (n_players <= 1);
            

            for(int i=1; i < n_players + 1; i++)
            {
                Console.Write("Enter the name for the player {0}: ", i);
                string nickName = Console.ReadLine();

                players.Add(new CPlayer(nickName, customBoard));
            }

            string pressed = "";
            int count = 0;

            do
            {
                if (count >= n_players)
                    count = 0;

                CPlayer currentPlayer = players[count];
                Console.WriteLine("It's the player {0}'s turn", currentPlayer.NickName);
                Console.WriteLine("Press R to Roll the die or A to abort the game.");
                pressed = Console.ReadLine();

                if(pressed.Equals("R", StringComparison.CurrentCultureIgnoreCase)) {

                    Console.WriteLine("--------------------------------");

                    currentPlayer.Roll();
                    Console.WriteLine("Dice's result: {0} ", currentPlayer.DiceResult);

                    int previousPosition = currentPlayer.Position;
                    currentPlayer.Move();
                    Console.WriteLine("You moved from cell [{0}] ====> to cell [{1}]", previousPosition, currentPlayer.Position);

                    if (currentPlayer.Winner)
                    {
                        Console.WriteLine("Player {0} won the game.", currentPlayer.NickName);
                        break;
                    }

                    Console.WriteLine("--------------------------------");
                    count++;
                }

            } while (!pressed.Equals("A", StringComparison.CurrentCultureIgnoreCase));

            Console.ReadLine();
        }
    }
}

Based on the code, I'd like to know what I could improve, what mistakes I did, what advice you'd give me, in a nutshell: How could I improve the code and be better at OOP. Thanks!

\$\endgroup\$
1
  • \$\begingroup\$ How did you connect the classes to the form, like how did you create the players themselves in a game board? You can send the code of the form.cs \$\endgroup\$ Commented Apr 21 at 14:12

2 Answers 2

6
\$\begingroup\$
  • When an array is created, it is nulled automatically. Array.Clear(board, 0, board.Length); is obsolete.

  • Rolling the dice: You are waiting a few milliseconds before creating the Random object to get different seeds. Better: Create it as a static field. The effect is that only one Random object will be created (and seeded once), no matter how many players you create.

    class CPlayer
    {
        private static readonly Random rnd = new Random();
    
        public void Roll()
        {
            diceResult = rnd.Next(1, 7);
        }
        ...
    }
    
  • We can also mark board as readonly since it is set only in the constructors. Note: readonly refers to the reference stored in board, not the contents of the board.

  • You are using dictionaries for the definition of snakes and ladders; however, you are never doing a dictionary lookup. Dictionaries are fast when looking up a value by key either through the indexer or the method TryGetValue. Inserting into a dictionary involves some overhead. Better use a list or an array.

    E.g., you could create an array of KeyValuePair or ValueTuple:
    (int from, int to)[] ladders = { (2, 38), (7, 14),… };

  • You are testing whether ladders and snakes are null twice. Do it once. You can still throw an exception after having added ladders and snakes to the board. This does not hurt. In C# 9.0 you can use the test is not null:

    if (ladders is not null) {
        ladderSize = ladders.Count;
        createSnakesOrLadders(ladders);
    }
    // TODO: Do the same for the snakes.
    
  • The calculation of the total number of snakes and ladders is wrong. It must be:

    if (ladderSize + snakesSize > board.Length / 2) {
        throw new Exception("The total sum of Snakes and Ladders cannot exceed 50% of the board.");
    }
    
  • In class CPlayer you are initializing position, nickName, diceResult and winner to their default values. Other than local variables, class fields are automatically initialized to their default values. You can do it if you think that it better reflects you intention, but it is not necessary.

  • The usual naming conventions for C# is to not prepend a C for class names. Often developers prepend field names with an underscore to better distinguish them from local variables.

  • Optional: You could simplify moving the token if instead of initializing the board with 0s, you initialized it with the indexes:
    for (int i = 0; i < board.Length; i++) { board[i] = i; }
    Then, no matter whether there is a snake or ladder or not:
    position = board[diceResult + position];

  • Another simplification would be to replace null initial snakes and ladders arrays by empty ones. This eliminates some checks and the need for storing the sizes in variables as you can then directly use the array lengths.

    public CBoard(int altura, int largo,
        (int from, int to)[] ladders = null, (int from, int to)[] snakes = null)
    {
        ladders = ladders ?? Array.Empty<(int, int)>();
        snakes = snakes ?? Array.Empty<(int, int)>();
        ...
    
  • Instead of testing i < n_players + 1, you can test i <= n_players.

  • You can use modulo arithmetic to make the player index turn around:
    count = (count + 1) % n_players;

  • You can use string interpolation:
    Console.WriteLine($"Player {currentPlayer.NickName} won the game.");
    is easier to read than
    Console.WriteLine("Player {0} won the game.", currentPlayer.NickName);

  • You can use auto properties in some cases. They automatically create an invisible backing field.

    private readonly string nickName;
    public string NickName { get => nickName; }
    

    Can be replaced by

    public string NickName { get; }
    

    Note that a getter-only property can be set in the constructor.

  • By using

    // pressed is declared as char
    pressed = Char.ToUpper(Console.ReadKey().KeyChar);
    Console.WriteLine();
    

    instead of

    pressed = Console.ReadLine();
    

    The user does not need to press Enter after entering R or A and you eliminate the need to do a complex comparison involving StringComparison.CurrentCultureIgnoreCase.

I suggested some changes using C# techniques that you might not be familiar with. You can just ignore them and keep your approach, if you prefer.

Here is a possible solution

class Board
{
    public int[] GameBoard { get; }

    public Board()
    {
        GameBoard = CreateBoard(100);
    }

    public Board(int altura, int largo,
        (int, int)[] ladders = null, (int, int)[] snakes = null)
    {
        if (altura < 2 || largo < 2) {
            throw new Exception("The height and length need to be at least greater than 1.");
        }

        // Ensure non-null arrays.
        ladders = ladders ?? Array.Empty<(int, int)>();
        snakes = snakes ?? Array.Empty<(int, int)>();

        GameBoard = CreateBoard(altura * largo);
        if (ladders.Length + snakes.Length > GameBoard.Length / 2) {
            throw new Exception("The total sum of Snakes and Ladders cannot exceed 50% of the board.");
        }

        CreateSnakesOrLadders(ladders);
        CreateSnakesOrLadders(snakes);
    }

    private int[] CreateBoard(int size)
    {
        int[] board = new int[size];
        for (int i = 0; i < size; i++) {
            board[i] = i;
        }
        return board;
    }

    private void CreateSnakesOrLadders((int, int)[] jumps)
    {
        foreach (var (from, to) in jumps) {
            GameBoard[from - 1] = to - 1;
        }
    }
}
class Player
{
    private static readonly Random _rnd = new Random();
    private readonly int[] _board;

    private int _position;
    public int Position => _position + 1;

    public int DiceResult { get; private set; }
    public string NickName { get; }
    public bool Winner { get; private set; }

    public Player(string nickName, Board board)
    {
        NickName = nickName;
        _board = board.GameBoard;
    }

    public void Roll()
    {
        DiceResult = _rnd.Next(1, 7);
    }

    public void Move()
    {
        if (_position + DiceResult < _board.Length) {
            _position = _board[DiceResult + _position];
            if (_position == _board.Length - 1) {
                Winner = true;
            }
        }
    }
}
class Program
{
    public static void Main_()
    {
        (int, int)[] ladders = {
            (2, 38), (7, 14), (8, 31), (16, 26), (21, 42),
            (28, 84), (36, 44), (51, 67), (71, 91), (78, 98), (87, 94)
        };

        (int, int)[] snakes = {
            (15, 5), (48, 10), (45, 24), (61, 18), (63, 59),
            (73, 52), (88, 67), (91, 87), (94, 74), (98, 79)
        };

        var board = new Board(10, 10, ladders, snakes);
        var players = new List<Player>();

        int numPlayers;
        do {
            Console.Write("Enter the number of players: ");
            numPlayers = Convert.ToInt32(Console.ReadLine());

            if (numPlayers <= 1) {
                Console.WriteLine("The total of players need to be 2 or more.");
            }
        } while (numPlayers <= 1);

        for (int i = 1; i <= numPlayers; i++) {
            Console.Write($"Enter the name for the player {i}: ");
            string nickName = Console.ReadLine();

            players.Add(new Player(nickName, board));
        }

        char pressed;
        int count = 0;
        do {
            Player currentPlayer = players[count];
            Console.WriteLine($"It's the player {currentPlayer.NickName}'s turn");
            Console.WriteLine("Press R to Roll the dice or A to abort the game.");
            pressed = Char.ToUpper(Console.ReadKey().KeyChar);
            Console.WriteLine();

            if (pressed == 'R') {
                Console.WriteLine("--------------------------------");

                currentPlayer.Roll();
                Console.WriteLine($"Dice's result: {currentPlayer.DiceResult} ");

                int previousPosition = currentPlayer.Position;
                currentPlayer.Move();
                Console.WriteLine($"{currentPlayer.NickName} moved from cell [{previousPosition}] ====> to cell [{currentPlayer.Position}]");

                if (currentPlayer.Winner) {
                    Console.WriteLine($"Player {currentPlayer.NickName} won the game.");
                    break;
                }

                Console.WriteLine("--------------------------------");
                count = (count + 1) % numPlayers;
            }
        } while (pressed != 'A');
    }
}

Q & A

Also, just to be sure, what would be the appropriate scenarios where you would use auto properties?

Auto properties were introduced in C# 3.0 and were improved in later versions. They aim to make your life easier. Use them when ever you can, i.e., when there is no logic other than getting or setting the backing field. In Position we cannot use it because we are returning _position + 1.


instead of var in the foreach statement, what'd be its type? I tried with (int,int)[] but it didn't work. Happened the same with (int, int)

Tuples can be deconstructed, i.e., their components can be extracted into variables. The foreach loop does it with var (from, to) in jumps. This could also be written as (int from, int to) in jumps. This declares two new int variables from and to and assigns them the tuple elements.

Alternatively, we could keep the tuple as is and write var jump in jumps or explicitly (int, int) jump in jumps and then access its components through the loop variable jump with jump.Item1 and jump.Item2. We can also give custom names to the tuple items: (int from, int to) jump in jumps and then access them with jump.from and jump.to.

Note that we are looping through an array (int, int)[] jumps, i.e., an array consisting of elements of the tuple type (int, int). Therefore, the loop variable must be a (int, int).

As you can see, we have a lot of options here: deconstruct vs. keeping the tuple, implicit versus explicit type declarations, using standard vs. custom tuple item names.


public int Position => _position + 1; this line is acting like a getter? It means that I can omit the get statement in that way?

This is quite a new addition to the language and is called “expression bodied members”. This is only a simplified syntax variant having no special meaning. It can be used whenever a method or a getter consists of a single return statement or when a void method, a setter or a constructor contains a single expression (where an assignment is an expression). Examples:

string GetGreeting() { return “Hello”; } same as string GetGreeting() => “Hello”;.

string Greeting { get { return “Hello”; } } same as string Greeting => “Hello”;.

string Greeting { get { return g; } set { g = value; } } same as
string Greeting { get => g; set => g = value; }.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks a lot for your reply @Olivier Jacot-Descombes, I feel that I've learnt a LOT from this. Thanks for such a good answer! :) \$\endgroup\$
    – Sharki
    Commented Oct 16, 2021 at 18:13
  • \$\begingroup\$ Also, just to be sure, what would be the appropriate scenarios where you would use auto properties? \$\endgroup\$
    – Sharki
    Commented Oct 16, 2021 at 18:25
  • \$\begingroup\$ And my last two questions, 1. instead of var in the foreach statement, what'd be its type? I tried with (int,int)[] but it didn't work. Happened the same with (int, int). 2. public int Position => _position + 1; this line is acting like a getter? It means that I can omite the get statement in that way? Thanks in advance, as I've said I've learnt a lot from this. \$\endgroup\$
    – Sharki
    Commented Oct 17, 2021 at 0:58
1
\$\begingroup\$

It would have been much more fun to play if you had some sort of visualization of what's happening on the board.

I have come up with this quick and dirty method, but I'm sure you can do much better!

public void PrintBoard(List<CPlayer> players)
{
    Console.Write("        ");
    for (int col = 0; col < 10; col++)
        Console.Write(col.ToString("D1") + "    ");
    Console.WriteLine("");
    for (int row = 9; row >= 0; row--)
    {
        Console.Write((row * 10).ToString("D2") + " | ");
        for (int col = 0; col < 10; col++)
        {
            int index = col + row * 10;
            char c = ' ';
            foreach (var player in players)
            {
                if (player.Position - 1 == index)
                    c = player.NickName[0];
            }
            Console.Write(board[index].ToString("D2") + "[" + c + "]");
        }
        Console.WriteLine();
    }
}

After another 5 minutes of playing, I realized that it might be better to play against someone else. I asked my wife to join but she was not interested.

Perhaps you should consider the option of playing against the computer if the number of players is 1.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Also I was referring to this post in the other one! :) I found funny that you asked your wife to play this awful code haha, thanks for your reply, I also found interesting to implement an option to play against the machine. Have a good day and sorry for the late response. \$\endgroup\$
    – Sharki
    Commented Nov 26, 2021 at 18:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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