As for the question, there's a lot to fix there. Firstly, if you're going to use auto-properties then you don't define your own backing fields because the auto-property does that for you. In the old days, we used to have to implement a property like this:
public class Player
{
private string _name;
public string Name
{
get
{
return _name;
}
set
{
_name = value
}
}
}
Nowadays, if all you do in the getter and setter is get and set the backing field, you can achieve the same result with this:
public class Player
{
public string Name { get; set; }
}
The system will implement the getter, the setter and the backing field behind the scenes. If you look at the private members of the type, you'll find that you can actually access the implicitly declared field named
_Name.
In the case of collection properties, you should pretty much always declare then as read-only and create the collection internally. What that means is that calling code can get the existing collection and then get, set, add and remove items but the existing collection cannot be replaced in its entirety:
public class Player
{
public string Name { get; set; }
public List<int> Turns { get; } = new List<int>();
}
That's how it is throughout the .NET Framework already, with properties like
Control.Controls,
ComboBox.Items,
DataTable.Rows, etc, etc.
What that means in your scenario is that, if you're not actually going to adding any turns in the collection, you don't need to care about that property in the constructor:
public class Player
{
public string Name { get; set; }
public bool PlayerIsComputer { get; set; }
public int RoundsPlayed { get; set; }
public List<int> Turns { get; } = new List<int>();
public Player(string name, bool playerIsComputer, int roundsPlayed)
{
Name = name;
PlayerIsComputer = playerIsComputer;
RoundsPlayed = roundsPlayed;
}
}
To be frank, I'm not sure why you would need the
roundsPlayed parameter either. Won't that always be zero for a new
Player? If so, there's no point using a parameter and you can just set the property explicitly to zero.
If there was a reason to pass in some existing values for
Turns then you could do it like this:
public class Player
{
public string Name { get; set; }
public bool PlayerIsComputer { get; set; }
public int RoundsPlayed { get; set; }
public List<int> Turns { get; } = new List<int>();
public Player(string name, bool playerIsComputer, int roundsPlayed)
{
Name = name;
PlayerIsComputer = playerIsComputer;
RoundsPlayed = roundsPlayed;
}
public Player(string name, bool playerIsComputer, int roundsPlayed, IEnumerable<int> turns)
: this(name, playerIsComputer, roundsPlayed)
{
Turns.AddRange(turns);
}
}
In that case, the second constructor invokes the first constructor to do the same thing with the first three parameters, then adds the
turns values to the existing collection. You could then create a new
Player like this:
players.Add(new Player(ShuffleName(), true, 0));
or like this:
players.Add(new Player(ShuffleName(), true, 0, new [] {1, 2, 3}));
Because the
turns parameter is type
IEnumerable<int>, you can pass any type that implements that interface, which includes a
List<int> or an
int array or many other less common options.
