Before a job interview, I was asked to create a program that can calculate the score of a bowling game. The full assignment and my code can be viewed on my Github. The company told me that this was not good enough, so I would like to get feedback about what's wrong and how to improve it.
Assignment
The game consists of 10 frames as shown above. In each frame the player has two opportunities to knock down 10 pins. The score for the frame is the total number of pins knocked down, plus bonuses for strikes and spares.
A spare is when the player knocks down all 10 pins in two tries. The bonus for that frame is the number of pins knocked down by the next roll. So in frame 3 above, the score is 10 (the total number knocked down) plus a bonus of 5 (the number of pins knocked down on the next roll.)
A strike is when the player knocks down all 10 pins on his first try. The bonus for that frame is the value of the next two balls rolled.
In the tenth frame a player who rolls a spare or strike is allowed to roll the extra balls to complete the frame. However no more than three balls can be rolled in tenth frame.
Requirements
Write a class named “Game” that has two methods
roll(pins : int)
is called each time the player rolls a ball. The argument is the number of pins knocked down.
score() : int
is called only at the very end of the game. It returns the total score for that game.
My solution
Program.cs
using System;
namespace BowlingScore
{
class Program
{
static void Main(string[] args)
{
var game = new Game();
game.Roll(1);
game.Roll(4);
game.Roll(4);
game.Roll(5);
game.Roll(6);
game.Roll(4);
game.Roll(5);
game.Roll(5);
game.Roll(10);
game.Roll(0);
game.Roll(1);
game.Roll(7);
game.Roll(3);
game.Roll(6);
game.Roll(4);
game.Roll(10);
game.Roll(2);
game.Roll(8);
game.Roll(6);
Console.WriteLine("Score: " + game.Score());
Console.ReadLine();
}
}
}
Game.cs
using System;
using System.Collections.Generic;
using System.Linq;
namespace BowlingScore
{
internal class Game
{
/// <summary>
/// Maximum number of Frames allowed in this game
/// </summary>
private const int MaxFrameCount = 10;
/// <summary>
/// Number of pins that each Frame should start with
/// </summary>
private const int StartingPinCount = 10;
private readonly List<Frame> frames = new List<Frame>();
private int score;
/// <summary>
/// Register the result of a roll
/// </summary>
/// <param name="knockedDownPins">How many pins have been knocked down</param>
public void Roll(int knockedDownPins)
{
if (frames.Count == MaxFrameCount && frames.Last().IsClosed)
{
throw new InvalidOperationException("You've played enough for today! Consider calling Score()");
}
if (!frames.Any() || frames.Last().IsClosed)
{
var isLastFrame = frames.Count == MaxFrameCount - 1;
frames.Add(new Frame(StartingPinCount, isLastFrame));
}
frames.Last().RegisterRoll(knockedDownPins);
}
/// <summary>
/// Get the total score
/// </summary>
/// <returns>The total score calculated from all Frames</returns>
public int Score()
{
for (var frameIndex = 0; frameIndex < frames.Count; frameIndex++)
{
var frame = frames[frameIndex];
var frameScore = 0;
var bonusScore = 0;
var isStrike = false;
// cap the roll index to 2 to avoid over-counting points if the last frame has bonus rolls
var maxRollIndex = frame.RollResults.Count < 2 ? frame.RollResults.Count : 2;
for (var rollIndex = 0; rollIndex < maxRollIndex; rollIndex++)
{
var result = frame.RollResults[rollIndex];
frameScore += result;
// calculate bonus score for a strike
if (result == StartingPinCount)
{
isStrike = true;
// look 2 rolls ahead
bonusScore += CalculateBonusScore(frameIndex, rollIndex, 2);
break;
}
}
// calculate bonus score for a spare
if (!isStrike && frameScore == StartingPinCount)
{
// look 1 roll ahead
bonusScore += CalculateBonusScore(frameIndex, maxRollIndex - 1, 1);
}
score += frameScore + bonusScore;
}
return score;
}
/// <summary>
/// Recursive function to calculate the bonus score of the next X rolls
/// </summary>
/// <param name="frameIndex">Index of the current frame</param>
/// <param name="rollIndex">Index of the current roll</param>
/// <param name="rollCount">How many rolls to look ahead</param>
/// <returns>The amount of bonus score calculated from the next X rolls</returns>
private int CalculateBonusScore(int frameIndex, int rollIndex, int rollCount)
{
if (rollCount == 0)
{
return 0;
}
var bonusPoints = 0;
// add the next roll in the same frame, if any
if (frames[frameIndex].RollResults.Count > rollIndex + 1)
{
bonusPoints += frames[frameIndex].RollResults[rollIndex + 1];
bonusPoints += CalculateBonusScore(frameIndex, rollIndex + 1, rollCount - 1);
}
else
{
// add the first roll of the next frame, if any
if (frames.Count > frameIndex + 1)
{
bonusPoints += frames[frameIndex + 1].RollResults[0];
bonusPoints += CalculateBonusScore(frameIndex + 1, 0, rollCount - 1);
}
}
return bonusPoints;
}
}
}
Frame.cs
using System;
using System.Collections.Generic;
namespace BowlingScore
{
internal class Frame
{
/// <summary>
/// How many pins have been knocked down in each roll
/// </summary>
public List<int> RollResults { get; } = new List<int>();
/// <summary>
/// No more rolls can be registered on a closed Frame
/// </summary>
public bool IsClosed => !isLastFrame && standingPins == 0 ||
!isLastFrame && RollResults.Count == 2 ||
RollResults.Count == 3;
private int standingPins;
private readonly int startingPinCount;
private readonly bool isLastFrame;
private bool extraRollAllowed;
/// <summary>
/// Create a new Frame
/// </summary>
/// <param name="startingPinCount">Number of pins that the Frame should start with</param>
/// <param name="isLastFrame">Special rules apply on the last frame</param>
public Frame(int startingPinCount, bool isLastFrame = false)
{
this.startingPinCount = startingPinCount;
standingPins = startingPinCount;
this.isLastFrame = isLastFrame;
}
/// <summary>
/// Register the result of a roll
/// </summary>
/// <param name="knockedDownPins">How many pins have been knocked down</param>
public void RegisterRoll(int knockedDownPins)
{
ValidateRoll(knockedDownPins);
RollResults.Add(knockedDownPins);
standingPins -= knockedDownPins;
ResetPinsIfNecessary();
}
private void ResetPinsIfNecessary()
{
if (isLastFrame && standingPins == 0)
{
standingPins = startingPinCount;
extraRollAllowed = true;
}
}
private void ValidateRoll(int knockedDownPins)
{
if (standingPins == 0)
{
throw new InvalidOperationException("Can't roll when there are no standing pins");
}
if (!isLastFrame && RollResults.Count == 2 ||
isLastFrame && RollResults.Count == 2 && !extraRollAllowed ||
RollResults.Count > 2)
{
throw new InvalidOperationException($"Can't register more than {RollResults.Count} rolls in this frame");
}
if (knockedDownPins < 0 || knockedDownPins > standingPins)
{
throw new InvalidOperationException($"Can't knock down {knockedDownPins} while there are only {standingPins} standing pins");
}
}
}
}