4
\$\begingroup\$

The rules of the game are pretty simple. I'll put it here in case people don't know them

It's a two player game hand game where they alternate being a batsman and a bowler. Who decides to do what initially is determined by a toss.

These are the hand gestures which has a corresponding score enter image description here

  • Both the batsman and the bowler throw their hands at the same time to one of the poses shown in the diagram.
  • The runs of each pose are added to the batsman’s score.
  • If both of them throw the same pose, the batsman is out and the score he scored is the target for the next batsman

I tried to re-create this game in the most OO way possible.

Here is my Player class

public class Player
{
    public string Name { get; set; }
    public int TotalScore { get; private set; }
    public List<int> Scores { get; private set; }
    private static Random random;
    internal CoinOptions coinOption;

    public Player(string name)
    {
        random = new Random();
        Name = name;
        Scores = new List<int>();
    }

    public int Play()
    {
        return random.Next(6);
    }

    internal void AddScore(int score)
    {
        this.TotalScore += score;
        this.Scores.Add(score);
    }

}

Coin class for the Toss

internal static class Coin
{
    private static Random random = new Random();
    public static CoinOptions FlipCoin()
    {
        var probability = random.Next(1);
        return (CoinOptions)probability;
    }
}

And the core logic in the HandCricketGame class

public class HandCricket
{
    Player playerOne, playerTwo;
    private Player batsman, bowler;
    private int inningsCount = 0;
    public HandCricket(Player one, Player two)
    {
        playerOne = one;
        playerTwo = two;
    }

    public GameResultSummary Play()
    {
        var tossResult = Toss();
        var gameResult = new GameResultSummary() { TossResult = tossResult };
        setBatsamanAndBowler(tossResult);
        gameResult.Innings.Add(new Innings() { Batsman = batsman, Bowler = bowler });
        playInnings();
        inningsCount += 1;
        switchBatsmanAndBowler();
        gameResult.Innings.Add(new Innings() { Batsman = batsman, Bowler = bowler });
        playInnings();
        gameResult.GameResult.DoesGameHaveResult  = true;
        if (batsman.TotalScore > bowler.TotalScore)
        {
            gameResult.GameResult.Winner = batsman;
        }
        else if (batsman.TotalScore < bowler.TotalScore)
        {
            gameResult.GameResult.Winner = bowler;
        }
        else
        {
            gameResult.GameResult.DoesGameHaveResult = false;
        }
        return gameResult;
    }

    private TossResult Toss()
    {
        playerOne.coinOption = CoinOptions.Heads;
        playerTwo.coinOption = CoinOptions.Tails;
        var winningFlip = Coin.FlipCoin();
        var winningPlayer = winningFlip == playerOne.coinOption ? playerOne : playerTwo;
        return new TossResult() { TossWinningPlayer = winningPlayer, TossWinningPlayerSelection = PlayingOptions.Bat };
    }
    private void playInnings()
    {
        int batsmanScore, bowlingScore;
        while(true)
        {
            batsmanScore = batsman.Play();
            bowlingScore = bowler.Play();
            if((batsmanScore == bowlingScore) || (inningsCount > 0 && batsman.TotalScore > bowler.TotalScore))
            {
                break;
            }
            batsman.AddScore(batsmanScore);
        }
    }

    private void switchBatsmanAndBowler()
    {
        var tempObject = batsman;
        batsman = bowler;
        bowler = tempObject;
    }

    private void setBatsamanAndBowler(TossResult tossResult)
    {
        if(tossResult.TossWinningPlayerSelection == PlayingOptions.Bat)
        {
            batsman = tossResult.TossWinningPlayer;
            bowler = getOpponentPlayerObject(tossResult.TossWinningPlayer);
        }
        else
        {
            bowler = tossResult.TossWinningPlayer;
            batsman = getOpponentPlayerObject(tossResult.TossWinningPlayer);
        }
    }

    private Player getOpponentPlayerObject(Player firstPlayer)
    {
        if (ReferenceEquals(playerOne, firstPlayer))
        {
            return playerTwo;
        }
        return playerOne;
    }
}

GameResultSummary Class

public class GameResultSummary
{
    internal TossResult TossResult { get;  set; }
    internal GameResult GameResult { get; set; }
    internal List<Innings> Innings { get;  set; }
    public GameResultSummary()
    {
        Innings = new List<Innings>();
        GameResult = new GameResult();
    }
    public void GameSummary()
    {
        Console.WriteLine(string.Format("Player {0} won the toss and elected to {1}", TossResult.TossWinningPlayer.Name, TossResult.TossWinningPlayerSelection.ToString()));
        foreach (var player in Innings)
        {
            Console.WriteLine(string.Format("Player {0} played and scored {1}", player.Batsman.Name, player.Batsman.TotalScore));
        }
        if(GameResult.DoesGameHaveResult)
        {
            Console.WriteLine(string.Format("{0} won the game", GameResult.Winner.Name));
        }
        else
        {
            Console.WriteLine(string.Format("Game was drawn"));
        }
    }


}

GameResult class

class GameResult
{
    internal Player Winner { get; set; }
    internal bool DoesGameHaveResult { get; set; }
}

Innings Class

public class Innings
{
    public Player Batsman { get; set; }
    public Player Bowler { get; set; }
}

Toss Result Class

public class TossResult
{
    public Player TossWinningPlayer { get; set; }
    public PlayingOptions TossWinningPlayerSelection { get; set; }
}

And a bunch of Enums

public enum CoinOptions
{
    Heads = 0, 
    Tails = 1
}
public enum PlayingOptions
{
    Bat = 0, Bowl = 1
}

The handCricketGame and the Player class are the most important ones here. What started as an OO approach I feel looks less like it when it is done.

The majority of it has to do with the Play method. Could you please suggest on how it can be written in a more OO way

\$\endgroup\$
3
\$\begingroup\$

Using Better Abstractions

Player Class

The Player class has properties that do not really belong there. The Scores property is more suited as a property in the HandCricketGame class since the player can exist without playing a game thus making these fields redundant and it's better if which player and how he scored is tracked from the main HandCricketGame class.

So is the coinOptions property which would really depend on the game being played.

Abstracting this to an interface we have

public interface IPlayer
{
    string Name { get; }
    int Play();
}

And it's implementations like

class Player : IPlayer
{
    public string Name
    {
        get; private set;
    }
    public Player(string name)
    {
        Name = name;
    }
    public int Play()
    {
        return RandomNumberGenerator.Play();
    }
}

Innings Class

Since we realized that storing the player score is better suited in the game object rather than the player object. We can go one step further and store it in the Innings object. The game is composed of multiple innings and it would make sense to store which player was the batsman, bowler and how the runs were scored in the Inning object in the HandCricketGame rather than directly on the HandCricketGame class.

Which will look like

public interface IInnings 
{
    IPlayer Batsman { get;  }
    IPlayer Bowler { get; }
    int TotalScore { get; }
    List<int> ScoreByEachThrow { get;}
    void AddScore(int score);
}

class Innings : IInnings
{
    public IPlayer Batsman
    {
        get;
        private set;
    }

    public IPlayer Bowler
    {
        get;
        private set;
    }

    public int TotalScore
    {
        get;
        private set;
    }

    public Innings(IPlayer batsman, IPlayer bowler)
    {
        Batsman = batsman;
        Bowler = bowler;
        ScoreByEachThrow = new List<int>();
    }

    public List<int> ScoreByEachThrow
    {
        get;
        private set;
    }

    public void AddScore(int score)
    {
        ScoreByEachThrow.Add(score);
        TotalScore += score;
    }
}

Single Responsibility Principle

The HandCricketGame is breaking the Single responsibility principle by doing multiple things like implementing the Tossing logic as well as playing the game. This class would look cleaner if this toss logic was moved to another class and that class used as a dependency to the HandCricketGame class.

public class TossCore
{
    public TossResult Toss(IPlayer playerOne, IPlayer playerTwo)
    {
        var callingTossOption = TossOption.Heads;
        var winningTossOption = RandomNumberGenerator.CoinToss();
        var winner = callingTossOption == winningTossOption ? playerOne : playerTwo;

        //Assuming player one alwayas calls heads and the toss and chooses to bat first
        return new TossResult(tossCalledByPlayer : playerOne, tossCalledOption : callingTossOption, tossWinningPlayer : winner, 
            winningTossOption : winningTossOption,
            winningPlayerOption : PlayingOptions.Bat
            );
    }
}

public class TossResult
{
    public IPlayer TossCalledByPlayer { get; private set; }
    public TossOption TossCalledOption { get; private set; }
    public IPlayer TossWinningPlayer { get; private set; }
    public TossOption WinningTossOption { get; private set; }
    public PlayingOptions WinningPlayerOption { get; set; }

    public TossResult(IPlayer tossCalledByPlayer, TossOption tossCalledOption, IPlayer tossWinningPlayer, 
        TossOption winningTossOption, PlayingOptions winningPlayerOption)
    {
        TossCalledByPlayer = tossCalledByPlayer;
        TossCalledOption = tossCalledOption;
        TossWinningPlayer = tossWinningPlayer;
        WinningTossOption = winningTossOption;
        WinningPlayerOption = winningPlayerOption;
    }
}

The Play method in the HandCricketGame class is very structural and could be modularized to make this method look clean and more readable.

Since we already know there are only going to be 2 Innings in the game it would be better to store both of them as individual objects rather than as a List.

public class HandCricketGame
{
    private readonly IPlayer playerOne, playerTwo;
    private IPlayer batsman, bowler;
    private IInnings firstInning, secondInning;
    public TossResult TossResult { get; private set; }

    private readonly TossCore TossCore;

    public HandCricketGame(IPlayer one, IPlayer two, TossCore toss)
    {
        playerOne = one;
        playerTwo = two;
        TossCore = toss;
    }

    public GameResult Play()
    {
        Toss();
        firstInning = PlayInning();
        SwapPlayerRoleAfterInnings();
        secondInning = PlayInning();
        return GameSummary();
    }

    private void Toss()
    {
        TossResult = TossCore.Toss(playerOne, playerTwo);
        if(TossResult.WinningPlayerOption == PlayingOptions.Bat)
        {
            batsman = TossResult.TossWinningPlayer;
            bowler = GetOpponentPlayerReferenceObject(batsman);
        }
        else
        {
            bowler = TossResult.TossWinningPlayer;
            batsman = GetOpponentPlayerReferenceObject(bowler);
        }
    }

    private GameResult GameSummary()
    {
        var hasResult = true;
        IPlayer winner = null;
        if (firstInning.TotalScore == secondInning.TotalScore)
        {
            hasResult = false;
        }
        else if (firstInning.TotalScore > secondInning.TotalScore)
        {
            winner = firstInning.Batsman;
        }
        else
        {
            winner = secondInning.Batsman;
        }
        var result = new GameResult(tossResult : TossResult,hasResult: hasResult, firstInning: firstInning, secondInning: secondInning, winner: winner);
        return result;
    }

    private IInnings PlayInning()
    {
        var currentInnings = new Innings(batsman, bowler);
        int currentScore = 0;
        int batsmanPlay, bowlerPlay;
        do
        {
            batsmanPlay = batsman.Play();
            bowlerPlay = batsman.Play();
            if (batsmanPlay != bowlerPlay)
            {
                currentScore += batsmanPlay;
                currentInnings.AddScore(batsmanPlay);
            }
        } while (batsmanPlay != bowlerPlay && (firstInning == null || currentScore <= firstInning.TotalScore));

        return currentInnings;
    }

    private IPlayer GetOpponentPlayerReferenceObject(IPlayer player)
    {
        if (ReferenceEquals(player, playerOne))
        {
            return playerTwo;
        }
        return playerOne;
    }

    private void SwapPlayerRoleAfterInnings()
    {
        IPlayer tempBatsman = batsman;
        batsman = GetOpponentPlayerReferenceObject(batsman);
        bowler = tempBatsman;
    }
}
\$\endgroup\$

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.