Search code examples
c#.netcode-structure

C# Throw Exception and catch in the same method why it's bad?


Have some discussion about one implementation:

// Pseudocode
accessor type GetValue() 
{
    try
    {
        do some action with possible throw exception1
        do some action with possible throw exception2

        return value;
    }
    catch (Exception ex)
    {
        value = default;
        throw Wraped in Meaningfull Exception ex

    }
}

Can someone explain why it might be a bad design, to use try-catch like that (throw and catch at the same method) to safely do some actions and agregate different type of similar exception?


Solution

  • It's not rethrowing

     throw new WrapedException("MyNewMessage", ex);
    

    that is wrong but catching all the exceptions

     catch (Exception ex) {
       ...
     }
    

    is a bad design: it masks potentially dangerous behaviour. Let's see why. Suppose we use GetValue() like this:

       try {
         someValue = GetValue();
       }
       catch (WrapedException) {  
         // We failed to obtain someValue;
         // The reason - WrapedException - is innocent
         // Let's use default value then
    
         someValue = defaultSomeValue;
       }
    

    And the actual picture is

       public GetValue() {
         try {
           do some action with possible throw exception1
    
           // Catastrophy here: AccessViolationException! System is in ruins!
           do some action with possible throw exception2
    
           return value;
         }
         catch (Exception ex) { // AccessViolationException will be caught...
           // ...and the disaster will have been masked as being just WrapedException
           throw new WrapedException("MyNewMessage", ex);
         }
       }
    

    Your design is OK if you catch only expected exception types:

       public GetValue() {
         try {
           do some action with possible throw exception1
           do some action with possible throw exception2
    
           return value;
         }
         catch (FileNotFound ex) {
           // File not found, nothing special in the context of the routine
           throw new WrapedException("File not found and we can't load the CCalue", ex);
         }
       }