views:

954

answers:

3

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

+2  A: 

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);
}
Matt Campbell
Possibly, but then you have the memory of the questions dictionary plus some list of keys - which is redundant. Thanks for the quick response though!
David Ackerman
So... where are you getting the keys from *now*?
Matt Campbell
A: 

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.

keithwarren7
+2  A: 

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?
Jon Skeet
I really appreciate the insight - I am very new to .NET in general, coming from a C++ background, so this would explain my unconventional thinking. Comments like these are great for my learning.
David Ackerman