is logging an exception on the code of the Exceptions base class a good design approach?

Question

some friends just finished the implementation of an app and they use Custom Exceptions. something that took my attention is that when a custom exception was raised they logged the exception in the code of the exception base class they implemented. so my question will be is this a good design approach?. my thinking is that a logging helper is more usable

 public class BaseCustomException: System.Exception
{

   public BaseCustomException()
   {
              TightlyCoupledClass.Log(this);
   }

}

Answer 1

No, this is a horrible approach IMO. The reason being is that the assumption is that every exception that is created is going to be thrown, which is definitely not the case.

Since Exceptions typically are read only, there are implementations where one exception will be created for a particular situation, and rethrown as necessary (the CLR sets the StackTrace when the exception is thrown, not when constructed).

Bottom line, it's not common, but it is possible that Exceptions are not thrown when created, and you shouldn't make assumptions based on that.

Answer 2

I don't think so. I'd set a global exception handler that would write the exception (and other data) to the log. That way you're only writing caught exceptions.

Answer 3

I understand their thinking in putting this into place so that they can assure that logging happens whenever an exception of CustomException is thrown; however, this is definitely smelly code.

An exception should be used just for exceptions and the code that executes the code that could cause a CustomException to be thrown should be able to decide what to do with that exception and whether or not to log it... because the logging should be specific to the scenario in which it was caused.

A side note, custom exceptions should inherit from ApplicationException as a root so that you can tell if the exception is custom to your business libraries or from the .NET framework itself.
--CORRECTION-- I just found this post that I was not aware of that essentially renders using ApplicationException useless.

Answer 4

Logging within the constructor is plain nasty. How else is the code that was attempting the operation supposed to construct a meaningful message that might include the exception ? The role of the exception is to represent the problem not log a message. Some other code should take care of that so that every part that catching that particular exception can do what they wish which may or may not involve logging the exception.

In the case of your log inside the constructor if the exception was wrapped and rethrown to be caught again the exception would be logged twice which is dumb.

catch ( CustomException exception ){ Logger.error( "Really useful message", exception ); }

Answer 5

This solution is too tightly coupled, and not configurable. I would recommend using a more robust exception handling framework, such as the Enterprise Library Exception Handling Application Block.