Flaw in SolutionContexts.GetEnumerator

I’m guessing that this post will be useful to a very small number of people if at all. However there might be a bigger lesson here for people writing their own collections and that is how not to code GetEnumerator method.

but first some background information.

when working with a visual studio add-in I needed find out weather a specific project is excluded from the build cycle.

the DTE provide interfaces and object to access this info and the code I ended up with looked like this:

   1: foreach (SolutionContext context in 

   2:    ApplicationObject.Solution.SolutionBuild.

   3:        ActiveConfiguration.SolutionContexts)

   4: {

   5:     if (context.ProjectName.Contains(project.Name))

   6:         return !context.ShouldBuild;

   7: }

However, this code resulted in an exception popping from time to time which took me a little while to understand.

the exception I got was :

System.ArgumentException
The parameter is incorrect. (Exception from HRESULT: 0x80070057 (E_INVALIDARG))
System.Collections.IEnumerator GetEnumerator()
at EnvDTE.SolutionContexts.GetEnumerator()

where the line number pointed to the foreach statement.

After some digging I found that this happens if in the solution you have an unloaded project as part of the solution. and the fix was quite simple jut replace the foreach with a simple for resulting in something like:

   1: var contexts = ApplicationObject.Solution.

   2:    SolutionBuild.ActiveConfiguration.

   3:    SolutionContexts;

   4:  

   5: for (int i = 0; i < contexts.Count; i++)

   6: {

   7:     try

   8:     {

   9:         var context = contexts.Item(i + 1);

  10:         if (context.ProjectName.Contains(project.Name))

  11:             return !context.ShouldBuild;

  12:     }

  13:     catch (ArgumentException)

  14:     {

  15:         //this is thrown when a project is unloaded

  16:     }

  17: }

  18: return false;

Now I can understand why an unloaded project will throw an exception if you try to access its configuration data. What I don’t understand is why they didn’t handle this inside the GetEnumerator and instead choose to crash it as well.

I think this is a mistake, in general if a piece of the information is missing don’t fail the entire operation just exclude that piece from the result. that way you will enable the user to decide what to do with the info he does have.

what do you think?

Posted in: IDE