HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Two-player number guessing game

Submitted by: @import:stackexchange-codereview··
0
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("---------"

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:

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.