Avoiding "enumeration modified" Exception

[alex21]

For my program i need a great deal of completely random, unique, 12 character id's.

So i wrote these three functions

 
public static string GiveUniqueID(IEnumerator id_key, int count)
{
string id = "";
bool done = false;
while (!done)
{
id = CreateUniqueID(id_key, count);
 
if (id != "@@@")
done = true;
}
 
return id;
}
 
public static string CreateUniqueID(IEnumerator id_key, int count)
{
List<string> ids = new List<string>();
for(int i = 0; i < count; ++i)
{
try
{
id_key.MoveNext();
ids.Add((string)id_key.Current);
}
catch (Exception)
{
return "@@@";
}
}
 
bool done = false;
string id = "";
while (!done)
{
id = UniqueID();
 
if (!ids.Contains(id))
done = true;
}
 
return id;
}
 
public static string UniqueID()
{
string toreturn = Session.IdPrefix;
for (int i = 0; i != 10; ++i)
toreturn += Utility.RandomList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0).ToString();
 
return toreturn;
}

So when i need a unique id i simply go string s = GiveUniqueID(); , but every while and then the program will freeze because the exception "enumeratio n modified" keeps repeating in an endless loop.

I am passing the Keys.GetEnumera tor(); of dictionaries of varying value types and the count of the dictionary. Since the dictionary's are of varying types i cannot simply pass the dictionary as an argument.

So what i am asking is how to avoid this endless loop?

[vekipeki]

Well, first of all, you should never catch an exception just to ignore it. Remove the try/catch block because you actually want to break the program flow when something goes wrong.

Passing strings with special meanings such as "@@@" is not a very good practice, it will make your code really hard to read, so you should also avoid it.

Are you sure that your exception is "Enumeratio n modified"? It does not seem that you are actually modifying the enumeration.

My suggestion is to use an IDictionary parameter instead of IEnumerator, it will give you .Count and .Contains(), so you don't have to copy it to a new list every time. Or you can at least use IEnumerable instead of IEnumerator to simplify your code a bit.

Try something like this:

public static string CreateUniqueID(IDictionary dictionary)
{
    String id = UniqueID();

    // Check if our dictionary already contains this key.
    while (dictionary.Contains(id))
        id = UniqueID();

    return id;
}

You can also consider using System.Guid class, it generates random GUIDs so you don't have to do it yourself:

String guid = Guid.NewGuid().ToString();

And please use the CODE tags when posting ! :o)

[Plater]

I was wondering what happens if you call that function again before the first one is done?

[alex21]

Its a multi thread program, which primarily operates in system memory but uses a database to communicate and synchronize with other instances of the program over the network, so the thread that is synchronizing system data with the database is modifying the dictionary I'm trying to find a unique id for, and to prevent duplicate id's from this i give each client a 2 character id prefix such as "aa".

i managed to fix this problem however simply by making sure it does not endlessly loop, however the problem i have run into now is that the synchronization process is extremely slow, as to read the results of a query the synchronization thread waits on the query thread, this results in only one query being executed while i wait for the result, where as what im aiming for is the execution of many queries without waiting for results.

[vekipeki]

If your ID doesn't have to be a 10-character string, it would be easiest to simply use:

String id = Guid.NewGuid().ToString();

This will give you a unique ID every time, without a need for any checks - but it is actually be a hex representation of a 128-bit number.

If you have a really large dictionary, then you will obviously spend some time looking for a unique 10-digit ID (Birthday Paradox is a good sample of collision statistics).

Also, if you are still using try/catch, note that it is one of the largest performance bottlenecks - each caught exception really slows down execution.

[Plater]

Only the generation of new unique IDs has to be done "one at a time" right? So its ok for your other queries to happen all the time?
Consider only waiting when it deals with the ID generation?