none
enhancing try catch code RRS feed

  • Question

  • using (Transaction acTrans = acDB.TransactionManager.StartTransaction()) { try { // request for object to be selected in the drawing area PromptSelectionResult selection = editor.GetSelection(selectionFilter); if the prompt status is OK, onjects were selected if (selection.Status != PromptStatus.OK ) { editor.WriteMessage("wtf there was No valid selection "); acTrans.Dispose(); } SelectionSet selecionSet = selection.Value; // Check to make sure a valid SelectedObject object was returned. if (selecionSet == null) { editor.WriteMessage("wtf there was No selection "); acTrans.Dispose(); } // end the transaction acTrans.Commit(); } catch (Autodesk.AutoCAD.Runtime.Exception ex) { editor.WriteMessage(ex.Message); } finally { acTrans.Dispose(); } }

    is that code uses try catch correctrly?
    Saturday, March 9, 2019 9:51 PM

Answers

  • Also, apart from the Dispose in the "finally" block, you are doing acTrans.Dispose() within the "if" blocks in the "try".

    So you are currently executing three consecutive calls to acTrans.Dispose: First, within the "try" block. Then, the "finally" takes over and does a second Dispose. And then the "using" takes over and does a third Dispose. That's superfluous, you only need to call Dispose once. Let the "using" block do it and remove all the other calls to acTrans.Dispose.

    Also, there is a big mistake there: Your "if" blocks call acTrans.Dispose but they don't exit the method. Execution flows down and it reaches the call to acTrans.Commit. In general this is wrong, you should not be making any calls to an object after having disposed it. Well, you could write a class in such a way that certain methods are still useful after calling Dispose, but in general it would be a bad idea since this would not be what the callers of the class expect.

    Sunday, March 10, 2019 3:24 PM
    Moderator

All replies

  • You don't need the "finally" block - that's what the "using" does.

    The "try/catch" is fine though.

    Alternatively, you could leave the "finally" block in and remove the "using" part.


    Convert between VB, C#, C++, & Java (http://www.tangiblesoftwaresolutions.com)
    Instant C# - VB to C# Converter
    Instant VB - C# to VB Converter

    Sunday, March 10, 2019 3:19 AM
  • Also, apart from the Dispose in the "finally" block, you are doing acTrans.Dispose() within the "if" blocks in the "try".

    So you are currently executing three consecutive calls to acTrans.Dispose: First, within the "try" block. Then, the "finally" takes over and does a second Dispose. And then the "using" takes over and does a third Dispose. That's superfluous, you only need to call Dispose once. Let the "using" block do it and remove all the other calls to acTrans.Dispose.

    Also, there is a big mistake there: Your "if" blocks call acTrans.Dispose but they don't exit the method. Execution flows down and it reaches the call to acTrans.Commit. In general this is wrong, you should not be making any calls to an object after having disposed it. Well, you could write a class in such a way that certain methods are still useful after calling Dispose, but in general it would be a bad idea since this would not be what the callers of the class expect.

    Sunday, March 10, 2019 3:24 PM
    Moderator
  • Also, you should have a generic catch in case something causes another type of exception to be thrown.

    catch (Autodesk.AutoCAD.Runtime.Exception ex)
    {
        editor.WriteMessage(ex.Message);
    }
    catch (Exception ex)
    {
        editor.WriteMessage(ex.Message);
    }

    When "stacking" Exceptions like this, always put the more specific exceptions first, and the last one should be the "catch-all" (pun intended).


    ~~Bonnie DeWitt [C# MVP]

    http://geek-goddess-bonnie.blogspot.com


    Sunday, March 10, 2019 4:48 PM
    Moderator