Question

I have searched for what could be causing the unexpected output (for me at least) and I haven't got very far. I think it has something to do with classes being a reference type, but I'm confused as you'll see the output below shows the strings as I expect however the contents of the array seems to be updating a reference rather then a value? I'm confused. I've stripped my code down to the following. Basically I have an unknown number of "players", with many more properties then I've shown here. Each "player" has pieces of "equipment", in actual fact there are always 24 pieces.

I've used an array here because I want to be able to compare the equipment for different players at the same index in the array.

public class Equipment
{
    public int PropA { get; set; }
    public int PropB { get; set; }

    public Equipment ()
    {
        PropA = 0;
        PropB = 0;
    }

    public Equipment (Equipment eq)
    {
        PropA = eq.PropA;
        PropB = eq.PropB;
    }
}

public class Player
{
    public string FirstName { get; set; }
    public string LastName { get; set; }

    public Equipment[] Equip = new Equipment[2];

    public Player ()
    {
        FirstName = "";
        LastName = "";

        for (int i=0; i<Equip.Length; i++)
        {
            this.Equip[i] = new Equipment();
        }
    }

    public Player (Player p)
    {
        FirstName = p.FirstName;
        LastName = p.LastName;

        for (int i=0; i<p.Equip.Length; i++)
        {
            this.Equip[i] = p.Equip[i];
        }
    }
}

public void LoadPlayers()
{
    Player tmpPlayer = new Player();

    List<Player> PlayerList = new List<Player>();

    tmpPlayer.FirstName = "One";
    tmpPlayer.LastName = "Two";

    tmpPlayer.Equip[0].PropA = 1;
    tmpPlayer.Equip[0].PropB = 2;

    tmpPlayer.Equip[1].PropA = 3;
    tmpPlayer.Equip[1].PropB = 4;

    PlayerList.Add(new Player(tmpPlayer));

    tmpPlayer.FirstName = "Three";
    tmpPlayer.LastName = "Four";

    tmpPlayer.Equip[0].PropA = 5;
    tmpPlayer.Equip[0].PropB = 6;

    tmpPlayer.Equip[1].PropA = 7;
    tmpPlayer.Equip[1].PropB = 8;

    PlayerList.Add(new Player(tmpPlayer));

    foreach (Player p in PlayerList)
    {
        Console.WriteLine(p.FirstName);
        Console.WriteLine(p.LastName);

        Console.WriteLine(p.Equip[0].PropA.ToString());
        Console.WriteLine(p.Equip[0].PropB.ToString());
        Console.WriteLine(p.Equip[1].PropA.ToString());
        Console.WriteLine(p.Equip[1].PropB.ToString());
    }
}

This is the output I get:

One Two 5 6 7 8 Three Four 5 6 7 8

This is the output I expected:

One Two 1 2 3 4 Three Four 5 6 7 8

Other than pointing out my error is there an obvious better way of achieving my goal?

Thank you in advance, I appreciate any and all help!

Was it helpful?

Solution

You need to change

public Equipment[] Equip = new Equipment[1]; 

To public Equipment[] Equip = new Equipment[2]; otherwise you will get an IndexOutOfRangeException.

In your LoadPlayers method, you are modifiying the same instance of Player instead of creating a new player.So you get the same output because you just have one instance.Create a new instance then add it to your list.

Player tmpPlayer = new Player();

/* set the properties */

PlayerList.Add(new Player(tmpPlayer));

tmpPlayer = new Player();

/* set the properties */

PlayerList.Add(new Player(tmpPlayer));

And you should get your expected output.

OTHER TIPS

Hi the problem is that you only set one instance of the object tmpPlayer and just overrode the values within the object the second time you tried to set its parameters.

What you could try instead of making one object then changing it the whole time and running into memory reference issues you can place a new object into your array each time and set its properties as you add the object. You should be able to achieve it like so

PlayerList.Add(new Player()
{
    FirstName = "One";
    LastName = "Two";

    Equip[0].PropA = 1;
    Equip[0].PropB = 2;

    Equip[1].PropA = 3;
    Equip[1].PropB = 4;
});

PlayerList.Add(new Player()
{
    FirstName = "Three";
    LastName = "Four";

    Equip[0].PropA = 5;
    Equip[0].PropB = 6;

    Equip[1].PropA = 7;
    Equip[1].PropB = 8;
});

With this each object should be unique and hopefully you'll get the output you desired.

The problem is that your copy constructor, public Player (Player p), assigns the equipment of the copied player to the new player. Since you use this constructor to create the two players you actually put into your list, their equipments both refer to that of the original player tmpPlayer.

You could fix this by having your copy constructor perform a copy of the equipment:

public Player (Player p)
{
    FirstName = p.FirstName;
    LastName = p.LastName;

    for (int i=0; i<p.Equip.Length; i++)
    {
        this.Equip[i] = new Equipement(p.Equip[i]);
    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top