0
\$\begingroup\$

I'm creating my own application exception types so I could catch them higher in the call stack and take specific action. I'm looking to get feedback on the latest exception I've written for two reasons:

  1. I'm using the Exception snippet from Visual Studio 2013, and am wondering if it is any good and/or has potential problems;
  2. The main parameter for this Exception type would be a string, and I'm wondering what to do with the parameters. Currently I'm following the built in ArgumentException constructor-signatures-wise, but I fear future users of my exception will fall into the same trap as me with ArgumentException: providing a single argument with the paramName, instead of a message.

PS. I've checked the source for ArgumentException and explicitly decided not to put in the extra work of saving the tenantName in a field + property analogously, because the calling code does not yet need it. (Though I'm happy to get feedback on that choice as well of course.)

Bottom line: anything to improve on this custom exception?

[Serializable]
public class TenantNotFoundException : Exception
{
    public TenantNotFoundException() 
    { }

    public TenantNotFoundException(string message)
        : base(message)
    { }

    public TenantNotFoundException(string message, string tenantName)
        : base(string.Format("Tenant '{0}' not found. {1}", tenantName, message))
    { }

    public TenantNotFoundException(string message, Exception inner)
        : base(message, inner)
    { }

    public TenantNotFoundException(string message, string tenantName, Exception inner) 
        : base(string.Format("Tenant '{0}' not found.", tenantName), inner)
    { }

    protected TenantNotFoundException(SerializationInfo info, StreamingContext context)
        : base(info, context) 
    { }
}

PS. I was almost tempted to write just a single constructor with a string parameter for tenant name, but then I wondered (a) if people would be confused and enter a message instead of a tenant name, and (b) why the snippet suggests all the other constructors. This is what prompted me to post this question.


For reference, this is what you start with as the Visual Studio snippet:

[Serializable]
public class MyException : Exception
{
    public MyException() { }
    public MyException(string message) : base(message) { }
    public MyException(string message, Exception inner) : base(message, inner) { }
    protected MyException(
      SerializationInfo info,
      StreamingContext context)
        : base(info, context) { }
}
\$\endgroup\$
2
\$\begingroup\$

Using strongly typed data avoids this situation.

One easy and simple way to fix this argument confusion would be passing the user defined datatype instead of string which is reserved for message.

In TenantException sample, if you have Object model for Tenant, pass that object and let the constructor does the remaining with that.

[Serializable]
public class TenantNotFoundException : Exception
{
    public TenantNotFoundException() 
    { }

    public TenantNotFoundException(string message)
        : base(message)
    { }

    public TenantNotFoundException(string message, Tenant tenant)
        : base(string.Format("Tenant '{0}' not found. {1}", tenant.Name, message))
    { }
}
\$\endgroup\$
5
  • \$\begingroup\$ Ah yes, I had thought of that, but currently can't because that would require a large refactor of the base application. Even if I could, there would be a GetTenant(string tenantName) method somewhere that will still have the same problem. \$\endgroup\$
    – Jeroen
    Feb 3 '16 at 7:34
  • 1
    \$\begingroup\$ I would also recommend the usage of the already existing ObjectNotFoundException. The reason why is that exceptions per si are exceptional, creating you own exception should be than very very rare. If you are not up to using System.Data then you can go with InvalidOperationException \$\endgroup\$ Feb 3 '16 at 10:12
  • \$\begingroup\$ @BrunoCosta I disagree strongly with that recommendation. For one, that requires taking a dependency on System.Data.Entity both in the code throwing the exception as well as the consumer catching it. In addition, it will make my own specific exceptional domain situation indistinguishable from when underlying code might throw such an exception when a data source throws it. - Creating your own exceptions should not be "very very rare", at least not for the reason you mention: "that exceptions are exceptional" doesn't mean you have few exceptional situations in your domain. \$\endgroup\$
    – Jeroen
    Feb 4 '16 at 7:12
  • \$\begingroup\$ @Jeroen I'll go more simply about it then. Can you handle your exception without having a specific type for it? If yes, then you should use InvalidOperationException. If not you are allowed to create your own exception. What bothers me most is that you are creating an exception type just to format a message and you are not really adding anything else relevant to it. So you could simply create a method for that would throw an InvalidOperationException with that message. \$\endgroup\$ Feb 4 '16 at 8:11
  • \$\begingroup\$ @BrunoCosta "you are creating an exception just to format a message" ... you incorrectly summarize my intent. In my question I explicitly mention I'm creating the exception so I can specifically catch that type higher in the call stack. Reusing a framework exception defeats that entirely. \$\endgroup\$
    – Jeroen
    Feb 4 '16 at 8:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.