David Ackerman
David Ackerman

Reputation: 12729

Hopefully simple question about modifying dictionaries in C#

I have a huge dictionary of blank values in a variable called current like so:

struct movieuser {blah blah blah}
Dictionary<movieuser, float> questions = new Dictionary<movieuser, float>();

So I am looping through this dictionary and need to fill in the "answers", like so:

for(var k = questions.Keys.GetEnumerator();k.MoveNext(); )
{
    questions[k.Current] = retrieveGuess(k.Current.userID, k.Current.movieID);
}

Now, this doesn't work, because I get an InvalidOperationException from trying to modify the dictionary I am looping through. However, you can see that the code should work fine - since I am not adding or deleting any values, just modifying the value. I understand, however, why it is afraid of my attempting this.

What is the preferred way of doing this? I can't figure out a way to loop through a dictionary WITHOUT using iterators.

I don't really want to create a copy of the whole array, since it is a lot of data and will eat up my ram like its still Thanksgiving.

Thanks, Dave

Upvotes: 0

Views: 1468

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1503419

Matt's answer, getting the keys first, separately is the right way to go. Yes, there'll be some redundancy - but it will work. I'd take a working program which is easy to debug and maintain over an efficient program which either won't work or is hard to maintain any day.

Don't forget that if you make MovieUser a reference type, the array will only be the size of as many references as you've got users - that's pretty small. A million users will only take up 4MB or 8MB on x64. How many users have you really got?

Your code should therefore be something like:

IEnumerable<MovieUser> users = RetrieveUsers();

IDictionary<MovieUser, float> questions = new Dictionary<MovieUser, float>();
foreach (MovieUser user in users)
{
    questions[user] = RetrieveGuess(user);
}

If you're using .NET 3.5 (and can therefore use LINQ), it's even easier:

IDictionary<MovieUser, float> questions = 
    RetrieveUsers.ToDictionary(user => user, user => RetrieveGuess(user));

Note that if RetrieveUsers() can stream the list of users from its source (e.g. a file) then it will be efficient anyway, as you never need to know about more than one of them at a time while you're populating the dictionary.

A few comments on the rest of your code:

  • Code conventions matter. Capitalise the names of your types and methods to fit in with other .NET code.
  • You're not calling Dispose on the IEnumerator<T> produced by the call to GetEnumerator. If you just use foreach your code will be simpler and safer.
  • MovieUser should almost certainly be a class. Do you have a genuinely good reason for making it a struct?

Upvotes: 2

keithwarren7
keithwarren7

Reputation: 14278

store the dictionary keys in a temporary collection then loop over the temp collection and use the key value as your indexer parameter. This should get you around the exception.

Upvotes: 0

Matt Campbell
Matt Campbell

Reputation: 1152

Is there any reason you can't just populate the dictionary with both keys and values at the same time?

foreach(var key in someListOfKeys)
{
    questions.Add(key, retrieveGuess(key.userID, key.movieID);
}

Upvotes: 2

Related Questions