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:
- I'm using the
Exception
snippet from Visual Studio 2013, and am wondering if it is any good and/or has potential problems; - 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 withArgumentException
: providing a single argument with theparamName
, instead of amessage
.
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) { }
}
1 Answer 1
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))
{ }
}
-
\$\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– Jeroen2016年02月03日 07:34:26 +00:00Commented Feb 3, 2016 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\$Bruno Costa– Bruno Costa2016年02月03日 10:12:46 +00:00Commented Feb 3, 2016 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– Jeroen2016年02月04日 07:12:42 +00:00Commented Feb 4, 2016 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 anInvalidOperationException
with that message. \$\endgroup\$Bruno Costa– Bruno Costa2016年02月04日 08:11:34 +00:00Commented Feb 4, 2016 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– Jeroen2016年02月04日 08:19:06 +00:00Commented Feb 4, 2016 at 8:19