C# issue storing objects into Dictionaries

Question

This seems to be simple, but I'm not sure what I'm doing wrong...

I wrote the following Class, which I further use it in a Dictionary<string, ClassName> object, as its .Value:

public class StorageList
{
    static List<int> listForStorage = new();

    public void AddToListForStorage(int number)
    {
        listForStorage.Add(number);
    }

    public List<int> GetList()
    {
        return listForStorage;
    }
}

When I run the application, I create a Dictionary<string, StorageList> and add a few elements to it:

static void Main(string[] args)
{    
    Dictionary<string, StorageList> dictionary = new();
    dictionary.Add("idOne", new StorageList());
    dictionary.Add("idTwo", new StorageList());
    dictionary["idOne"].AddToListForStorage(1);
    dictionary["idOne"].AddToListForStorage(2);
    dictionary["idTwo"].AddToListForStorage(3);
    dictionary["idTwo"].AddToListForStorage(4);
}

When printing to the console either 'idOne' or 'idTwo', I was expecting to see 1 and 2 for 'idOne' and 3 and 4 for 'idTwo. However, I see 1, 2, 3 and 4 for both 'idOne' and 'idTwo'...

foreach (var id in dictionary)
{
    foreach (var item in dictionary[id.Key].GetList())
    {
        Console.WriteLine(item);
    }
    Console.WriteLine($"Finished {id.Key}");       
}
// 1
// 2
// 3 <-- Not expected
// 4 <-- Not expected
// Finished idOne
// 1 <-- Not expected
// 2 <-- Not expected
// 3
// 4
// Finished idTwo

Objects are different, so I don't quite understand why this is happening.

Console.WriteLine(Object.ReferenceEquals(dictionary["idOne"], dictionary["idTwo"]));
// false

I'd appreciate some assistance on this. Thanks!

Your `listForStorage` field is static. It shouldn't be. (Static means "relating to the type, not to any specific instance of the type" whereas you want a field for each separate instance of `StorageList`) — Jon Skeet, Dec 02 '21 at 14:11

Johnathan Barclay · Answer 1 · 2021-12-02T14:34:41.290

3

You have declared listForStorage as static, so it belongs to the StorageList type rather than any specific instance of StorageList.

Consequentially, there will be only one List<int> instance used by all instances of StorageList.

That being said, you probably want to make listForStorage an instance variable (remove the static keyword):

public class StorageList
{
    List<int> listForStorage = new();
}

Now each instance of StorageList will have its own listForStorage.

edited Dec 02 '21 at 14:34

answered Dec 02 '21 at 14:12

Johnathan Barclay

18,599
1
22
35

1

I find "shared by all instances" to be a confusing way of explaining static... it sounds like there has to be at least one instance. Instead, I prefer to explain it as "related to the type rather than to any specific instance of the type". – Jon Skeet Dec 02 '21 at 14:12
1

@JonSkeet Fair enough; I haven't given a definition of `static` though, just explained the consequence of using it in this context, which I feel would be more helpful to the OP. – Johnathan Barclay Dec 02 '21 at 14:15
Jeez! Two days trying to figure it out... Thanks!! – roccaforte Dec 02 '21 at 14:16
1

@JohnathanBarclay: Well, you've said "so it is shared by all instances of StorageList" - which implies some need for at least one StorageList IMO. I'm suggesting rewording it to a form that doesn't have that implication, e.g. "You have declared `listForStorage` as `static` so it is related to the `StorageList` type rather than each individual instance of `StorageList` having its own independent field". (Or something like that.) I'm taking the time to say this in the hope that you'll avoid the "shared" terminology in future explanations too. (It's a pity IMO that VB used Shared.) – Jon Skeet Dec 02 '21 at 14:21
1

@JonSkeet OK; reworded. – Johnathan Barclay Dec 02 '21 at 14:35

C# issue storing objects into Dictionaries

1 Answers1