4

Scenario:

I have a loop that iterates over an Array of COM objects and does some work using them.

My fear, working with COM objects, is that some exception will creep up (possibly on another user's environment) and cause the loop to break mid-way through processing these objects.

So, let's say (hypothetically) an exception occurs on file 32 of 173.

I see three options:

  1. Show the exception, inform the user that only 32 files were processed, and we then have to manually do the work on the remaining 141 files.

  2. Undo all the work done on the first 32 files, inform the user the request couldn't be completed and show the exception. This is less than ideal because by the time we are at this procedure, a lot of up-front work has been done (creating files, incrementing serial numbers, etc.) which would be cumbersome and difficult to undo. In the end, we end up with all 172 files needing manual work.

  3. Catch the exception, store it in a List, continue to try to process the rest of the files, then after trying to process all files, inform the user of any exceptions that occurred and any files which couldn't be processed. Best case we only have one file to manually do work on, and worst case we are left with 141 files - same as the first option.


Event horizon:

I'm new to programming. In fact I just wrote (am writing) my first add-in which I posted up on SE::Code Review.

Now, I'm of the mindset that option three makes the most sense - let's at least give hope a chance and see what happens with the rest of them, no?

So here's my implementation (well simplified, but this is the pattern):

Sub Main()
    Dim Exceptions as New List(of Exception)
    Try
        TryThatFunkyCOMthing (Exceptions)

        If Exceptions.Count>0 Then
            For Each ex As Exception in Exceptions
                MessageBox.Show(ex.ToString)
            Next
        End If

    Catch ex As Exception
        MessageBox.Show(ex.ToString)
    End Try
End Sub

Private Sub TryThatFunkyCOMthing (ByRef ExceptionBuffer as List(of Exception))
    For each COMthingy in COMthings
        Try
           COMthingy.DoSomething
        Catch ex As SpecificException
            COMthingy.WeAreBlewedUp
        Catch ex As Exception
            ExceptionBuffer.Add(ex)
        End Try
    Next
End Sub

One of the reviewers on CR gave me some feedback saying I should NOT be doing this. He said I should want the loop to stop if an exception occurs.

He linked to this SO Q&A regarding handling general exceptions, which I read (a few times now) and I think this solution falls under "failing gracefully". As mentioned above, I'm not trying to swallow exceptions, just control how/when they are handled so that my program behaves in a manner I can predict and depend on, even in the event of a catastrophic or unexpected exception.


What I'd like to know:

  1. If this is bad practice, why?
  2. Out of the three options I provided, which would you choose? - OR - Is there a mysterious and much better fourth option I'm missing?
2
  • 1
    Assuming all three options are valid with respect to your "business requirements", I'd agree that #3 is almost always the nicest from the user's point of view. The only real disadvantage imho is that it's a bit more complicated to implement (and if you make a silly mistake, exceptions might end up swallowed).
    – Ixrec
    Commented Jul 29, 2015 at 23:01
  • 1
    You will be absolutely fine with #3, whoever said that it is not advisable failed to understand your situation. Just try to append all the message texts of the exceptions into one big message, and show it all at once, instead of showing a different message box for each exception. Otherwise, in the event that every single one of your files has a problem, the user will have to click "OK" 173 times. Better yet, open up a modeless box at the beginning of the processing, and append to it the text of each exception as (and if) it happens.
    – Mike Nakis
    Commented Jul 29, 2015 at 23:29

2 Answers 2

5

I understand why you want to do this. The way you are doing it is not what I would do.

Rather than collect a list of exception objects, I would process those objects as they happen. By collecting a list to process later you run the risk of losing them before they get processed. At that point, without intending to do so, your program has swallowed the exceptions. The user will know nothing bad has happened. When they find out, the hard way, their experience with your program will be a poor one.

Whatever you want to do with the exceptions, do it as soon as they happen. Continue the loop if you like but process/log those exceptions regardless.

8
  • How could I "lose" them before I could process them? What could happen once I have an exception object in the list that would cause it to be "lost"? Would it be better to create a new List within the procedure scope, do my Try loop, and then use Throw to re-throw any exceptions caught before exiting the procedure? (Instead of returning them to the calling procedure)
    – CBRF23
    Commented Jul 29, 2015 at 23:14
  • 1
    While it may not be apparent form the beginning, programs grow and change over time. Some have many different developers in their lifetime. I grant that the window for losing your list is currently small, however, it can easily grow as more changes are made to your program. What you see as unlikely now may bite your backside in 18 months and 3 versions later. Yes, in your example, the window small. It would be smaller if there was no window. Not processing/logging the exception as it happens is a bad practice. That's all I'm trying to say.
    – WillG
    Commented Jul 29, 2015 at 23:25
  • Thanks Will - that is helpful and I understand what you are saying about programs growing/changing. I see what you are saying about keeping it simple and reducing any possible place for me/someone else to make changes that cause the list to get "lost' (e.g. forgetting to implement some handling of the list in the main procedure, etc.).
    – CBRF23
    Commented Jul 29, 2015 at 23:27
  • 1
    It would actually be fairly easy for you to lose that list if any exception besides COMException popped up. Part of my review was that you were catching and swallowing any and every exception instead of handling it.
    – RubberDuck
    Commented Jul 30, 2015 at 0:10
  • 2
    @CBRF23 that's exactly the problem. What if it's an OutOfMemory error? What does your program do then? You're casting your net too wide. Some exceptions are not your fault, you cannot prevent them, and you cannot sensibly clean up from them.
    – RubberDuck
    Commented Jul 30, 2015 at 11:55
4

You could also provide the best of both worlds, and just let the user decide what he prefers.

There are many software that provides through their settings these features:

  • stop on error
  • ignore error
  • an application-level undo

The first two options being mutually exclusive should be presented to the user using radio buttons.

For the undo/redo, a web search brought out the following (untested):

Implementing these 3 options shouldn't significantly bloat your method.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.