In the C# Player's Guide book there's a challenge in which you have to write the PigDice game in C#:
- It's a multiplayer game, every player starts with score 0.
- Player X rolls 2 dice.
- Player X's score is increased by the sum of the 2 dice' numbers.
- If Player X gets 1 on either die, his score becomes 0 again and it's the next player's turn.
- Player X can choose between play (roll dice) or hold (lose his turn)
- Player X wins if his score is equal or greater than N
Is my approach reasonable? How can I improve my program?
Program.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace PigDice
{
class Program
{
static void CreatePlayers(Player[] players)
{
for (int index = 0; index < players.Length; ++index)
{
string defaultName = $"Player #{index + 1}";
Console.Write($"{defaultName}'s name: ");
string playerName = Console.ReadLine();
players[index] = new Player(playerName == string.Empty ? defaultName : playerName);
}
}
static void ShowGameStatus(Game game) => Console.WriteLine(game.GetScoresTable());
static void Main(string[] args)
{
Console.WriteLine("Welcome to PigDice!");
Console.Write("Amount of players: ");
// Number of players that are going to play
string inputNumber = Console.ReadLine();
int numberOfPlayers = Convert.ToInt32(inputNumber);
Game game = new Game(numberOfPlayers);
Player[] players = game.Players;
// Ask for and assign players' names
CreatePlayers(players);
Console.WriteLine("All set. Let the games begin!");
// First player starts
game.PlayingPlayer = players[0];
// GameLoop returns false if ended
while (GameLoop(game)) ;
Console.WriteLine(game.PlayingPlayer + " has won!");
Console.ReadKey();
}
private static bool GameLoop(Game game)
{
Player player = game.PlayingPlayer;
ShowGameStatus(game);
if (player.hasWon())
return false;
Console.Write($"{player}'s turn! Play (no)? ");
string playersChoice = Console.ReadLine();
// Handle player's choice to hold
if (playersChoice.ToLower() == "no")
{
Console.WriteLine($"{player} chose to hold!");
game.NextTurn();
return true;
}
// Handle player's choice to play
Dice dice = new Dice();
Console.WriteLine($"{player} is rolling the dice!");
int number1 = dice.Roll();
int number2 = dice.Roll();
Console.WriteLine($"{player} got {number1} and {number2}");
// If one dice got 1, all score is lost
if (number1 == 1 || number2 == 1)
{
Console.WriteLine($"Bad luck! {player} lost all points!");
player.OverallScore = 0;
game.NextTurn();
return true;
}
// Player got a score increase
int scoreWon = number1 + number2;
player.OverallScore += scoreWon;
Console.WriteLine($"{player} got {scoreWon} score and now has {player.OverallScore}");
return true;
}
}
}
Player.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace PigDice
{
class Player
{
const int pointsToWin = 50; // instead of 100 which is
// incredibly hard to get
public string PlayerName { get; }
public int OverallScore { get; set; }
public bool hasWon() => this.OverallScore >= pointsToWin;
public Player(string playerName)
{
this.PlayerName = playerName;
}
public override string ToString()
{
return this.PlayerName;
}
}
}
Game.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace PigDice
{
class Game
{
public Player[] Players { get; }
public int Turn { get; private set; }
public Player PlayingPlayer { get; set; }
public Game(int numberOfPlayers)
{
Players = new Player[numberOfPlayers];
}
public void NextTurn()
{
Turn += 1;
PlayingPlayer = Players[Turn % Players.Length];
}
public string GetScoresTable() {
string table = "----- Scores -----";
foreach (Player player in Players)
table += "\n" + player + ": " + player.OverallScore;
return table;
}
}
}
Dice.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace PigDice
{
class Dice
{
private Random random = new Random();
public int Roll() => random.Next(6) + 1;
}
}
1 Answer 1
My initial impression is that there seems to be quite a lot of shared responsibility, particularly between the static methods in Program
and the rest of the code. For example:
private static bool GameLoop(Game game)
This really seems like it should belong in your Game
class. It's also odd that this function doesn't actually contain the loop, so you have to call it from within one:
while (GameLoop(game)) ;
This seems counter intuitive. Perhaps it should be GetRound
or something and called by GameLoop
where both of them sit in the Game
class...
Player Construction
Game
contains an array of players, which it creates on construction, however it isn't responsible for populating the players. This tightly couples the Game
class to CreatePlayers
in your Program
class. If GetScoresTable
is called before the players are created the program will crash.
A better approach might be to have CreatePlayers
actually create an array of players and then have the array passed into the constructor of Game
, rather than the number of players. That way, Game
is in a completely constructed state and is ready for other methods on the class to be called.
random.Next(6) + 1;
same asrandom.Next(1, 7);
\$\endgroup\$