4
\$\begingroup\$

This is my console simulation of the Slot Machine in C#. Please take a look at this code and point out possible ways of improvement.

using System;
using System.Linq;

namespace Casino
{
    class Program
    {
        public static void Main()
        {
            SlotMachine slotMachine = new SlotMachine();
            slotMachine.Play();
        }
    }
    class SlotMachine
    {
        private int _tapesCount = 3;
        private ConsoleColor _machineColor = ConsoleColor.Red; 
        
        public string[] Tapes { get; set; } = {"7", "J", "Q", "K", "A" } ;

        public void SetMachineColor(ConsoleColor color)
        {
            this._machineColor = color;
        }
        
        public string[] Play()
        {
            string[] result = new string[_tapesCount];
            
            Console.ForegroundColor = _machineColor;
            Console.Clear();
            
            Console.WriteLine(new string('-', _tapesCount+2));
            Console.WriteLine("|" + new string('7', _tapesCount) + "|");
            Console.WriteLine(new string('-', _tapesCount+2));
            var lastCursorPosition = Console.GetCursorPosition();
            
            Thread.Sleep(1000);
            
            for (int j = 0; j < _tapesCount; j++)
            {
                Random random = new Random();
                for (int i = 0; i < 50; i++)
                {
                    Console.SetCursorPosition(j+1, 1);
                    string currentTape = Tapes[random.Next(Tapes.Length)];
                    Console.Write(currentTape);
                    result[j] = currentTape;
                    Thread.Sleep(50);
                }
            }
            Console.SetCursorPosition(lastCursorPosition.Item1, lastCursorPosition.Item2);
            
            return result;
        }
    }
}
New contributor
K1BeR is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

Here are my refactoring suggestions:

SetMachineColor vs constructor

  • Depending on your use case it might make sense to set the _machineColor via the constructor rather via an extra setter method
public SlotMachine(ConsoleColor color = ConsoleColor.Red)
    => _machineColor = color;
  • With this setup you could mark the _machineColor as readonly
private readonly ConsoleColor _machineColor;

_tapesCount

  • As I can see this member never changes its value so it might make sense to mark it as const
private const int _tapesCount = 3;

Tapes

  • I think should change the underlying data type from string to char since in each tape you want to represent just a single character
public char[] Tapes { get; set; } = { '7', 'J', 'Q', 'K', 'A' };

Initial Screen

  • Your Play method has several responsibilities
    • It might make sense to extract some part of it into their own methods
  • Let's start with the drawing of the initial screen
private void DrawInitialScreen()
{
    Console.ForegroundColor = _machineColor;
    Console.Clear();

    var border = new string('-', _tapesCount + 2);
    Console.WriteLine(border);
    Console.WriteLine("|" + new string('7', _tapesCount) + "|");
    Console.WriteLine(border);
}
  • Since the top and bottom of the machine is the same you don't need to create two different strings for that

Spinning the tape

  • The next part which could be easily extracted is the spinning of the tape
private char SpinTape(int tape)
{
    var random = new Random();
    var currentTape = new char();
    for (int spin = 0; spin < 50; spin++)
    {
        Console.SetCursorPosition(tape, 1);
        currentTape = Tapes[random.Next(Tapes.Length)];
        Console.Write(currentTape);
            
        Thread.Sleep(50);
    }
    return currentTape;
}
  • Here I changed two things
    • Rather than updating the result array explicitly, this method returns the last character
    • I've renamed the loop's variable from i to spin to make it more meaningful

Put all these things together

public char[] Play()
{
    DrawInitialScreen();
    Thread.Sleep(1000);

    var (lastPosLeft, lastPosTop) = Console.GetCursorPosition();

    var result = new char[_tapesCount];
    for (int tape = 0; tape < _tapesCount; tape++)
        result[tape] = SpinTape(tape+1);

    Console.SetCursorPosition(lastPosLeft, lastPosTop);

    return result;
}
  • Here I've used deconstruction to store the lastCursorPosition's Left and Top components into separate variables
    • If you don't want to do that you can still use lastCursorPosition.Left instead of lastCursorPosition.Item1 and lastCursorPosition.Top instead of lastCursorPosition.Item2
  • I've changed here the result's data type from string to char to align with the Tapes
  • I've renamed the loop's variable from j to tape to make it more meaningful
  • If you want Play can return with a single string if call return result.ToString(); at the end of this method
\$\endgroup\$

Your Answer

K1BeR is a new contributor. Be nice, and check out our Code of Conduct.

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.