CODEWITHSUNDEEP
c#exceptioncustom-exceptionsnested-exceptions
Elephant
Elephant

Reputation: 727

Is it good practice to throw custom exception from catch statement?

I have command line tool which runs tests. There is test runner class, which does some preparation before test execution and then runs the test and makes a report. Is it OK if my classes catch an exception and throw new custom exception to the upper layer, and then the upper layer will throw it to the upper layer too (until the View class, which will display/log the exception)?

class Program
{
    static void Main(string[] args)
    {
        testRunner = new TestRunner();

        try
        {
            testRunner.RunTest();
            testRunner.GetReport();
        }
        catch (TestRunnerException ex)
        {
            Print(ex);  // prints nicely two levels of exceptions for user
            Log(ex);    // writes to file all levels of exceptions
        }
    }
}

class TestRunner
{
    void RunTest()
    {
        // run test here
    }

    TestReport GetTestReport()
    {
        try
        {
            return testReporter.GenerateReport();
        }
        catch (Exception ex)
        {
           throw new TestRunnerException("Failed to generate report.", ex);
        }
    }
}


class TestReporter
{
    TestReport GenerateReport()
    {
        try
        {
            // create report here
        }
        catch (Exception ex)
        {
            throw new ReportException($"Test '{testName}' missing data.", ex)
        }
    }
}

Upvotes: 0

Views: 2003

Answers (1)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186748

It's not throwing custom exception from catch but catching all exceptions that is a bad practice; imagine:

  TestReport GetTestReport() {
    // throws NullReferenceException (yes, GenerateReport() has a bug)
    return testReporter.GenerateReport(); 
  }
  catch (Exception ex) {
    // Bad Practice: here we hide hideous NullReferenceException, 
    // but throw innocent TestRunnerException
    throw new TestRunnerException("Failed to generate report.", ex);
  }

  ...

  try { 
    GetTestReport();
  }
  catch (TestRunnerException ex) {
    // Nothing serious: Tests report fails, let's do nothing
    // Under the hood: the routine went very wrong  - 
    // NullReferenceException - in GenerateReport(); 
    ;
  }

I suggest using specific exception(s) in the catch: 

  TestReport GetTestReport() {
    // throws NullReferenceException 
    return testReporter.GenerateReport(); 
  }
  catch (NotAuthorizedException ex) {
    // We are not allowed to run tests only, nothing serious
    throw new TestRunnerException("Failed to generate report. Not authorized", ex);
  }

  ...

  try { 
    GetTestReport();
  }
  catch (TestRunnerException ex) {
    // Nothing serious: Tests report fails, let's do nothing
    ;
  }

Upvotes: 1

Related Questions