patterncsharpMinor
Two-player number guessing game
Viewed 0 times
numberplayertwogameguessing
Problem
I created a 2-player number guessing game. Each player picks a number between 1 and 10 and closest one wins.
I'm just looking for best practices suggestions and improvements.
Console program.cs
```
using ClassLibrary1;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace GameConsole
{
class Program
{
static int[] _guesses = new int[2] { 0, 0 };
const int ERROR_INPUT = 1000;
const int ERROR_SAME = 1001;
static int _numberOfRounds = 2;
static Class1 _newGame;
static List _status = new List();
static void Main(string[] args)
{
_newGame = new Class1();
do
{
Console.Clear();
PrintScores();
PromptPlayer(Player.One);
PromptPlayer(Player.Two);
//both guesses are valid at this point
//Console.Clear();
//_status.ForEach(x => Console.WriteLine(x));
Player winner = _newGame.Play(_guesses);
Console.WriteLine("And the winner of this round was Player {0}. The number was: {1}", Enum.GetName(typeof(Player), winner), _newGame.TargetNumber);
Console.ReadKey();
//Setup new round
_guesses = new int[] { 0, 0 };
_status = new List();
_newGame.CreateNewTargetNumber();
} while (_newGame.PlayerOneScore Console.WriteLine(x));
if (_newGame.PlayerOneScore == _numberOfRounds)
Console.WriteLine("Player One wins!");
else
Console.WriteLine("Player Two wins!");
}
private static void PrintScores()
{
_status.Add(string.Format("Player One: {0}", _newGame.PlayerOneScore));
_status.Add(string.Format("Player Two: {0}", _newGame.PlayerTwoScore));
_status.Add("---------"
I'm just looking for best practices suggestions and improvements.
Console program.cs
```
using ClassLibrary1;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace GameConsole
{
class Program
{
static int[] _guesses = new int[2] { 0, 0 };
const int ERROR_INPUT = 1000;
const int ERROR_SAME = 1001;
static int _numberOfRounds = 2;
static Class1 _newGame;
static List _status = new List();
static void Main(string[] args)
{
_newGame = new Class1();
do
{
Console.Clear();
PrintScores();
PromptPlayer(Player.One);
PromptPlayer(Player.Two);
//both guesses are valid at this point
//Console.Clear();
//_status.ForEach(x => Console.WriteLine(x));
Player winner = _newGame.Play(_guesses);
Console.WriteLine("And the winner of this round was Player {0}. The number was: {1}", Enum.GetName(typeof(Player), winner), _newGame.TargetNumber);
Console.ReadKey();
//Setup new round
_guesses = new int[] { 0, 0 };
_status = new List();
_newGame.CreateNewTargetNumber();
} while (_newGame.PlayerOneScore Console.WriteLine(x));
if (_newGame.PlayerOneScore == _numberOfRounds)
Console.WriteLine("Player One wins!");
else
Console.WriteLine("Player Two wins!");
}
private static void PrintScores()
{
_status.Add(string.Format("Player One: {0}", _newGame.PlayerOneScore));
_status.Add(string.Format("Player Two: {0}", _newGame.PlayerTwoScore));
_status.Add("---------"
Solution
I see several things that could be changed/improved...
You dont't have to initialize array items if they should be zeros so this:
is the same as this
You write a comment above those three lines:
make it a method and the comment won't be necessary:
I think it would be a good idea to create a new
to encapsulate all those fields that hold data related to players like
Your enum then gets the name
This change will allow you to remove lot's of code (several fields) and make your classes simpler and easier to read.
So a line like this one:
would become:
or instead of hard coding player names or their count in strings
you'll have
and your application will be more flexible.
I guess the name of
and when I see this line:
its name should probably be
You dont't have to initialize array items if they should be zeros so this:
static int[] _guesses = new int[2] { 0, 0 };is the same as this
static int[] _guesses = new int[2];You write a comment above those three lines:
//Setup new round
_guesses = new int[] { 0, 0 };
_status = new List();
_newGame.CreateNewTargetNumber();make it a method and the comment won't be necessary:
private static void SetupNewRound()
{
_guesses = new int[2];
_status = new List();
_newGame.CreateNewTargetNumber();
}I think it would be a good idea to create a new
Player class and give it some properties like:class Player
{
public PlayerName Name { get; set; }
public int Guesses { get; set; }
public int Score { get; set; }
}to encapsulate all those fields that hold data related to players like
_guesses, PlayerOneScore, PlayerTwoScore, _scores etc. Currently player data is scattered over multiple classes and assemblies. You hold some of it in Class1 and _guesses in Program.Your enum then gets the name
PlayerName and you change _guesses to _players and make it a dictionary of players:Dictionary _players = new Players[]
{
Player { Name = PlayerName.One },
Player { Name = PlayerName.Two },
}.ToDictionary(p => p.Name);This change will allow you to remove lot's of code (several fields) and make your classes simpler and easier to read.
So a line like this one:
else if (_guesses[(int)player] == 0)would become:
else if (_players[playerName].Guesses == 0)or instead of hard coding player names or their count in strings
_status.Add(string.Format("Player One: {0}", _newGame.PlayerOneScore));you'll have
foreach(var player in _game.Players.Values)
{
status.Add(string.Format("Player {0}: {1}", player.Name, player.Score));
}and your application will be more flexible.
I guess the name of
Class1 is just an accident because you gave everything else very good names ;-)and when I see this line:
static Class1 _newGame;its name should probably be
Game.Code Snippets
static int[] _guesses = new int[2] { 0, 0 };static int[] _guesses = new int[2];//Setup new round
_guesses = new int[] { 0, 0 };
_status = new List<string>();
_newGame.CreateNewTargetNumber();private static void SetupNewRound()
{
_guesses = new int[2];
_status = new List<string>();
_newGame.CreateNewTargetNumber();
}class Player
{
public PlayerName Name { get; set; }
public int Guesses { get; set; }
public int Score { get; set; }
}Context
StackExchange Code Review Q#108560, answer score: 8
Revisions (0)
No revisions yet.