Calculation of the Bowling Game score

Question

I am new in developer role, and I have my code which I would like to simplify or make it better. And wondering if someone can do a quick check.

The code calculates the Score of the Bowling game, which is input as a string of the whole game (ex. as following: "X|7/|9-|x|-8|8/|-6|x|X|X||81").

Program.cs

 public class Program
{
    public static void Main(string[] args)
    {
        Console.WriteLine("#=======================#");
        Console.WriteLine("# Welcome to Ten-Pin Bowling! #");
        Console.WriteLine("#=======================#");

        var game = new TenPinGame("X|7/|9-|x|-8|8/|-6|x|X|X||81");
        Console.WriteLine("Game: {0}", input);
        Console.WriteLine("Score: " + game.Score());
        Console.WriteLine("#=====#");
        }

        Console.WriteLine("Thank you for playing with us!");
        Console.WriteLine("Have a nice day!");
        Console.Read();
    }
}

TenPinGame.cs

public class TenPinGame : IGame
{
    public const int MaxFrameNumber = 10;
    public const int StartingPinsNumber = 10;
    public List<Frame> Frames { get; } = new List<Frame>();

    private string[] framesStringArray;
    public static string bonusSubsctring;


    public TenPinGame (string gameInput)
    {
        string framesSubstring = ParseGame(gameInput);
        framesStringArray = Utilities.SplitFramesString(framesSubstring, MaxFrameNumber);
        FrameRepository.ProcessFrameString(framesStringArray, Frames, MaxFrameNumber, StartingPinsNumber);
    }

    /// <summary>
    /// Splitting input string into 2 parts: Frames and Bonus throws
    /// </summary>
    /// <param name="gameInput"></param>
    internal string ParseGame(string gameInput)
    {
        string framesSubstring = "";

        if (gameInput.Contains("||"))
        {
            int index = gameInput.IndexOf("||");
            framesSubstring = gameInput.Substring(0, index);
            bonusSubsctring = gameInput.Substring(index + 2);
        }
        return framesSubstring;
    }


    /// <summary>
    /// Calculate the Total score for the whole game/line.
    /// </summary>
    /// <returns></returns>
    public int Score()
    {
        int score = 0;
        foreach (var frame in Frames)
        {
            int frameIndex = Frames.IndexOf(frame);
            if (!frame.IsLastFrame && frame.IsStrike)
            {
                if (Frames[frameIndex + 1].Throws.Count == 1)
                    score += 10 + Frames[frameIndex + 1].Throws[0] + Frames[frameIndex + 2].Throws[0];
                else if (Frames[frameIndex + 1].Throws.Count == 2 || Frames[frameIndex + 1].Throws.Count == 3)
                    score += 10 + Frames[frameIndex + 1].Throws[0] + Frames[frameIndex + 1].Throws[1];
                //else
                //    throw new ArgumentException($"The next frame has invalid throws count {Frames[frameIndex + 1].Throws.Count}.");
            }
            else if (!frame.IsLastFrame && frame.IsSpare)
            {
                score += 10 + Frames[frameIndex + 1].Throws[0];
            }
            else if (frame.IsLastFrame && frame.IsStrike)
            {
                score += 10 + Frames[frameIndex].Throws[1] + Frames[frameIndex].Throws[2];
            }
            else if (frame.IsLastFrame && frame.IsSpare)
            {
                score += 10 + Frames[frameIndex].Throws[1];
            }
            else
            {
                score += frame.Throws[0] + frame.Throws[1];
            }
        }

        return score;
    }

}

Frame.cs

public class Frame
{
    public List<int> Throws = new List<int>();
    public int KnokcedDownPinsCount { get; set; }
    public bool IsStrike { get; set; }
    public bool IsSpare { get; set; }
    public bool IsLastFrame { get; set; }
    public bool IsBonusAllowed { get; set; }
    public bool IsFrameOver { get; set; }



    public Frame(bool isLastFrame = false)
    {
        this.IsLastFrame = isLastFrame;
    }


    public void ValidateEachThrow ()
    {
        if (TenPinGame.StartingPinsNumber == 0)
        {
            throw new InvalidOperationException("Sorry, no pins are left standing.");
        }
        if (KnokcedDownPinsCount < 0)
        {
            throw new InvalidOperationException($"Sorry, the # of knocked down pins {KnokcedDownPinsCount} is out of range.");
        }
        else if (KnokcedDownPinsCount > TenPinGame.StartingPinsNumber)
        {
            throw new InvalidOperationException($"Sorry, the # of knocked down pins {KnokcedDownPinsCount} is is more than {TenPinGame.StartingPinsNumber} still standing pins.");
        }
    }

}

Utilities.cs

public class Utilities
{


    /// <summary>
    /// Split each frame into a separate string in the string array
    /// </summary>
    /// <param name="framesString"></param>
    public static string[] SplitFramesString(string framesString, int maxFrameNumber)
    {
        string[] framesStringArray = Array.Empty<string>();
        if (framesString.Contains("|"))
        {
            framesStringArray = framesString.Split(new string[] { "|" }, StringSplitOptions.RemoveEmptyEntries);
            if (framesStringArray.Length > maxFrameNumber)
            {
                throw new ArgumentException($"Sorry, you have {framesStringArray.Length} frames, " +
                    $"which is higher than Max number of {maxFrameNumber} frames allowed for this game.");
            }
        }
        return framesStringArray;
    }

    /// <summary>
    /// Process each charcters in the given frame and set corresponding frame properties accordingly. 
    /// </summary>
    /// <param name="characterInString"></param>
    /// <param name="Frames"></param>
    /// <param name="stratingPinsNumber"></param>
    /// <returns></returns>
    public static int ProcessCharArray(char characterInString, Frame Frames, int stratingPinsNumber)
    {
        int pinsCount = 0;

        if (characterInString >= '0' && characterInString <= '9')
        {
            Frames.Throws.Add((int)char.GetNumericValue(characterInString));
            pinsCount = int.Parse(characterInString.ToString());
        }
        else if (characterInString.ToString().ToUpperInvariant() == "X")
        {
            Frames.IsStrike = true;
            Frames.IsFrameOver = true;
            Frames.Throws.Add(10);
            pinsCount = 10;
        }
        else if (characterInString.ToString().ToUpperInvariant() == "/")
        {
            Frames.IsSpare = true;
            Frames.IsFrameOver = true;
            Frames.Throws.Add(stratingPinsNumber - Frames.Throws[0]);
            pinsCount = 10;
        }
        else if (characterInString.ToString().ToUpperInvariant() == "-")
        {
            Frames.Throws.Add(0);
            pinsCount += 0;
        }
        else if (characterInString.ToString().ToUpperInvariant() == "/" && Frames.IsLastFrame && Frames.IsBonusAllowed)
        {
            throw new ArgumentException("The Spare cannot be set on the Bonus Throws, please check.");
        }
        else
        {
            throw new ArgumentException($"Invalid argument '{characterInString}' was detected in the provided input, please check.");
        }

        return pinsCount;
    }
}

FrameRepository.cs

public class FrameRepository
{
    /// <summary>
    /// Process each frame-string in the frames string array, to determine the pins count for each frame
    /// </summary>
    /// <param name="framesList"></param>
    public static void ProcessFrameString(string[] framesList, List<Frame> Frames, int MaxFrameNumber, int StartingPinsNumber)
    {
        foreach (var frame in framesList)
        {
            if (!Frames.Any() || Frames.Last().IsFrameOver)
            {
                var isLastFrame = Frames.Count == MaxFrameNumber - 1;
                Frames.Add(new Frame(isLastFrame));
            }

            char[] frameCharArr = frame.ToCharArray();
            foreach (char c in frameCharArr)
            {
                Frames.Last().KnokcedDownPinsCount = Utilities.ProcessCharArray(c, Frames.Last(), StartingPinsNumber);

                if (!Frames.Last().IsLastFrame && Frames.Last().Throws.Count == 2)
                {
                    Frames.Last().IsFrameOver = true;
                }
            }

            if (Frames.Count == MaxFrameNumber)
            {
                Frames.Last().IsLastFrame = true;
                char[] bonusCharArr = TenPinGame.bonusSubsctring.ToCharArray();
                if (Frames.Last().IsStrike)
                {
                    if (bonusCharArr.Length > 2)
                    {
                        throw new ArgumentException($"The number of bonus throws {bonusCharArr.Length}, is over allowed for the Last Strike. Please check.");
                    }
                    Frames.Last().IsBonusAllowed = true;
                    foreach (char c in bonusCharArr)
                    {
                        Frames.Last().KnokcedDownPinsCount = Utilities.ProcessCharArray(c, Frames.Last(), StartingPinsNumber);
                    }
                }
                else if (Frames.Last().IsSpare)
                {
                    if (bonusCharArr.Length != 1)
                    {
                        throw new ArgumentException($"The number of bonus throws {bonusCharArr.Length}, is over allowed for the Last Spare. Please check.");
                    }
                    Frames.Last().IsBonusAllowed = true;
                    Frames.Last().KnokcedDownPinsCount = Utilities.ProcessCharArray(bonusCharArr[0], Frames.Last(), StartingPinsNumber);
                }
            }
        }
    }
}

Answer 1

Something to note: you pass the overall game data into TenPinGame as a string, which you then parse. It would make sense to parse this data first, then instantiate the Game object.

Imagine if you had to add a new feature, which allowed users to input scores from a User Interface. You would then have two "access points", your TenPinGame class should reflect this.

From what I can see, there are three functions relating to input parsing: SplitFramesString, ProcessCharArray and ParseGame. It would make sense to move these three functions into a separate GameInputParser, like so:

public class GameInputParser
{
    public static string[] SplitFramesString(string framesString, int maxFrameNumber) { /*code*/ }

    public static int ProcessCharArray(char characterInString, Frame Frames, int stratingPinsNumber) { /*code*/ }

    internal string ParseGame(string gameInput) { /*code*/ }

} 

Answer 2

From a functional perspective, you could change your ProcessCharArray to stop allocating unneeded memory on the heap by using this. Being that a char is derivative of ValueType. The memory will be allocated on the stack within your defined scope and will keep the GC from cleaning up the mess that's created with characterInString.ToString().ToUpperInvariant() == "X".

public static int ProcessCharArray(char characterInString, Frame Frames, int 
stratingPinsNumber)
{
    int pinsCount = 0;

if (char.IsDigit(characterInString))
{
    Frames.Throws.Add((int)char.GetNumericValue(characterInString));
    pinsCount = int.Parse(characterInString.ToString());
}
else if (char.ToUpperInvariant(characterInString) == 'X')
{
    Frames.IsStrike = true;
    Frames.IsFrameOver = true;
    Frames.Throws.Add(10);
    pinsCount = 10;
}
else if (characterInString == '/')
{
    Frames.IsSpare = true;
    Frames.IsFrameOver = true;
    Frames.Throws.Add(stratingPinsNumber - Frames.Throws[0]);
    pinsCount = 10;
}
else if (characterInString == '-')
{
    Frames.Throws.Add(0);
    pinsCount += 0;
}
else if (characterInString == '/' && Frames.IsLastFrame && Frames.IsBonusAllowed)
{
    throw new ArgumentException("The Spare cannot be set on the Bonus Throws, please check.");
}
else
{
    throw new ArgumentException($"Invalid argument '{characterInString}' was detected in the provided input, please check.");
}

    return pinsCount;
}

here is an alternative approach for the logic in your method as well. :)

if (char.IsDigit(characterInString))
{
    Frames.Throws.Add((int)char.GetNumericValue(characterInString));
    var pinsCount = int.Parse(characterInString.ToString());
    return pinsCount;
}

switch(char.ToUpperInvariant(characterInString))
{
    case 'X':
        Frames.IsStrike = true;
        Frames.IsFrameOver = true;
        Frames.Throws.Add(10);
        return 10;
    case '-':
        Frames.Throws.Add(0);
        return 0;
    case '/':
        if(Frames.IsLastFrame && Frames.IsBonusAllowed)
            throw new ArgumentException("The Spare cannot be set on the Bonus Throws, please check.");

        Frames.IsSpare = true;
        Frames.IsFrameOver = true;
        Frames.Throws.Add(stratingPinsNumber - Frames.Throws[0]);
        return 10;
    default :  throw new ArgumentException($"Invalid argument '{characterInString}' was detected in the provided input, please check.");      
}