CODEWITHSUNDEEP
c#exception
TonyI
TonyI

Reputation: 155

What is the recommended way to catch exceptions

I have to do a code review and i got to a code part that addresses possibles exceptions. it looks to me that the developer coding works but i want to ask what is the usual and correct way to do that. What is the best way to do catch exceptions? the coder wrote:

  try 
  { . . . }
  catch (Exception ex)
  {
    if (ex is PlatformNotSupportedException)
    { //for the Windows version or edition that does not support.
    // tracing
    }
    else if (ex is NotSupportedException || ex is IOException)
    { // for the NTFS not supported or EFS is not configured
      // tracing
    }
    else
    {
      //report any exception as encrypt/decrypt
    }
  }

I thought that the book says that it should be:

  catch (PlatformNotSupportedException pnse)
  {
    //for the Windows version or edition that does not support.
    // tracing
  }
  catch (NotSupportedException nse)
  {
    // for the NTFS not supported or EFS is not configured
    // tracing
  }
  catch (IOException ioe)
  {
    // tracing for IOE
  }   
  catch (Exception e)
  {
      //report any exception as encrypt/decrypt
  }

Upvotes: 2

Views: 236

Answers (3)

Matthew Watson
Matthew Watson

Reputation: 109537

TLDR: Use the second form so that the compiler catches ordering errors.

The reason that you should use the second form is because then you will get a compile error if you attempt to handle the types in the wrong order.

For example, this will give you an actual compiler error:

try
{
    throw new ArgumentOutOfRangeException();
}

catch (Exception)
{
    Console.WriteLine("Caught 'Exception'");
}

// This gives a compile error:
// "Error CS0160  A previous catch clause already catches all exceptions of this or of a super type ('Exception')"

catch (SystemException)
{
    Console.WriteLine("Caught 'SystemException'");
}

However, using if/else if will NOT cause a compile error, so the error goes unnoticed:

try
{
    throw new ArgumentOutOfRangeException();
}

catch (Exception ex)
{
    if (ex is Exception)
    {
        Console.WriteLine("Caught 'Exception'");
    }
    else if (ex is SystemException) // This will never be reached, but no compile error.
    {
        Console.WriteLine("Caught 'SystemException'");
    }
}

Note, however, that tools such as Resharper will warn you for the second case.

Upvotes: 2

mubashar ilyas
mubashar ilyas

Reputation: 11

this would be generic for all type of exception

try 
{
   .....code
}
catch (Exception ex)
{
    MessageBox.Show(ex.Message);
}

Upvotes: 0

Darjan Bogdan
Darjan Bogdan

Reputation: 3900

The second approach would be more preferred. However, there is tiny difference between proposed solution and current one. You'd need to refactor to a method, or copy the code in two places (NotSupportedException and IOException catch blocks), whilst current implementation handles it under the same if block.

So, if you want to follow the same approach, you can use when keyword to filter out certain types and more.

  catch (PlatformNotSupportedException pnse)
  {
    // for the Windows version or edition that does not support.
    // tracing
  }
  catch (Exception ex) when (ex is NotSupportedException || ex is IOException)
  {
    // for the NTFS not supported or EFS is not configured
    // tracing
  } 
  catch (Exception e)
  {
      //report any exception as encrypt/decrypt
  }

If that's not mandatory, you can leave implementation as is

Upvotes: 6

Related Questions