A better fix for the failing OneNote automation script
I did not like my code fix for the error detection routine when trying to detect which file OneNote had loaded twice. To refresh my memory, here was what I initially checked in:
SortedList<string, string> loadedModules =
new SortedList<string, string>(StringComparer.InvariantCulture);
System.Diagnostics.Process onenote = System.Diagnostics.Process.GetProcessesByName("onenote")[0];
int i = 0;
Console.WriteLine("There are this many module loaded: " + onenote.Modules.Count);
foreach (System.Diagnostics.ProcessModule loadedModule in onenote.Modules)
{
Console.WriteLine ("Trying to add module " + i + " named " + loadedModule.ModuleName.ToString());
loadedModules.Add(loadedModule.ModuleName.ToLower(), loadedModule.FileName);
i++;
}
So what happens with this code is that the first time a module is detected as being loaded twice, the script will log the name of the module and then fail (exit). This is a decent fix, but has a big limitation from a test point of view.
Consider this list of animals:
Ant
Bat
Cat
Dog
Ant
Bat
Emu
Clearly, "ant" and "bat" are duplicated. If my logic was applied to detecting the duplicates, I would find out "ant" was duplicated, but not "bat" (since I error out after the first duplicate detected). So what would happen is I would spend time tracking down why I had a duplicated "ant," presumably come up with a fix and try to test the fix. If the same logic error was causing the "bat" problem, and if I had known about that extra error, I could fix the "bat" problem at the same time. But that is not what will happen here. After I fix the "ant" error, my test would notice "bat" was also duplicated. I could potentially need to restart my investigation for this new error and spend time recreating my fix.
There are differences in the two logging calls also - the first call which I added only tells me the name of the duplicated module, but the second also tells me the location from where the module was loaded. I'd like to have that information available each time a potential duplicate is being tested.
Investigating this problem is now becoming a bit harder. Since I did not know the "bat" duplication existed, I would need to spend some time determining if my recent changes to fix the "ant" problem had caused the "bat" problem, or if the "bat" problem had already existed. (On a tangent, if my "ant" fix caused the "bat" module to be duplicated, we call this a regression).
Clearly, a list of ALL duplicated files from the beginning would have helped.
So here is a much better fix for the loop:
foreach (System.Diagnostics.ProcessModule loadedModule in onenote.Modules)
{
Console.WriteLine ("Trying to add module " + i + " named " + loadedModule.ModuleName.ToString() +
"\nfrom location " + loadedModule.FileName );
try
{
loadedModules.Add(loadedModule.ModuleName.ToLower(), loadedModule.FileName);
}
catch (Exception e)
{
Console.WriteLine("Detected a duplicate module name with " +
loadedModule.ModuleName +
"load from location " +
loadedModule.FileName +
"\nThe error reported was " + e.ToString());
}
i++;
}
Now I know all modules that get duplicated and the location from which they were loaded.
Hard core coders will not like the try/catch commands, which is why I called this a "better" fix. A more optimal (from a speed point of view) could be checking the sorted list before trying to add the new element to see if it contains the element already. Quite frankly, though, that had not occurred to me, and I am not going to worry about the extra milliseconds this loop will consume. This code works, tells me what I need to know and helps me identify bugs. It is reliable, easy to read and easy to maintain. Optimizing for performance is not critical for this test, but would be the "best" fix.
Questions, comments, concerns and criticisms always welcome,
John