4

Is it good practice to inherit system exceptions besides Exception, if it makes sense? For example, would it be sensible to inherit System.InvalidOperationException, if my custom exception is specific kind of 'invalid operation'?

The reason I'm asking I have a number of classes that throw InvalidOperationException with the same message. I'm considering replacing these with a custom exception which can define its own default message. If it was to inherit the InvalidOperationException I would not have to refactor the catch handlers.

Dan Stevens
  • 6,392
  • 10
  • 49
  • 68
  • As long as it doesn't violate [Liskov's Substitution Principle](http://en.wikipedia.org/wiki/Liskov_substitution_principle) you should be fine. – Anthony Sep 23 '13 at 15:14
  • 2
    No, not particularly. You'd only ever use a custom exception type when you intend the exception to be caught and handled. In which case it doesn't matter which class is its base class, the catch clause must always use the specific type that you created. – Hans Passant Sep 23 '13 at 15:17

2 Answers2

4

No, short & simple reason for this is because InvalidOperationException derives from SystemException which according to the docs

Defines the base class for predefined exceptions in the System namespace.

It's considered best practise to derive from Exception.

Just to clarify - there is nothing wrong with deriving from InvalidOperationException (at least that I am aware of). However, deriving from ready-made exceptions like InvalidOperationException can result in you taking extra baggage that you may not need. I think the real question you need to ask is

"What exactly is it I am going to gain from deriving from InvalidOperationException over Exception"

From my personal experience, I have never really had to derive from anything other than Exception.

James
  • 80,725
  • 18
  • 167
  • 237
  • 1
    How does the quote specify that no other classes are allowed to derive from classes derived from `SystemException`? – Daniel Hilgarth Sep 23 '13 at 15:15
  • 1
    @DanielHilgarth "*predefined exceptions in the System namespace*" - to me that means they are geared towards *framework* specific exceptions. – James Sep 23 '13 at 15:16
  • 1
    "Should", "generally". Our man was indicating best practice, unless you want to dress your exception as a System exception -> code smell. – Gusdor Sep 23 '13 at 15:17
  • 1
    Geared towards, maybe, but I don't see any exclusivity here that would forbid own exceptions. – Daniel Hilgarth Sep 23 '13 at 15:19
  • 1
    @DanielHilgarth I never said it was forbidden - I am just pointing out that it's not considered "best practise". It's generally better to derive from `Exception` simply because you start from the base so you can design it to suit your specific needs. Deriving from ready-made exceptions like IOE (I assume) result in you taking extra baggage that you may not need. It does of course depend on the situation, however, from my personal experience I have never seen the need to do such things. – James Sep 23 '13 at 15:24
  • 1
    @James: And that comment is a way better explanation why you generally should derive from `System.Exception` directly than what you currently have in your answer. Because it really has nothing to do with `SystemException`. – Daniel Hilgarth Sep 23 '13 at 15:26
  • @DanielHilgarth fair point - I will update. However, I still think the point stands that anything which derives from `SystemException` are clearly *system* specific exceptions and therefore don't really have much use outwith that (just my opinion). – James Sep 23 '13 at 15:27
  • 1
    @James: Having a look at the [exception classes derived from IOE](http://msdn.microsoft.com/en-us/library/system.invalidoperationexception.aspx#inheritanceContinued), I tend to even disagree with this. To me not all of them are *"system specific"*, but that might be just my subjective POV. – Daniel Hilgarth Sep 23 '13 at 15:30
  • 1
    @DanielHilgarth yeah you could be right. I have found issues with how things are worded in the documentation in the past... However, I guess it doesn't really make much sense anyway - I mean what *really* are you gaining from deriving from such exceptions that you wouldn't get from `Exception`? I guess that's the point I am trying to make. – James Sep 23 '13 at 15:32
  • 1
    @James: It really depends on the exception you derive from. As with every other class, an exception class can implement features you want to reuse, for example the [Status](http://msdn.microsoft.com/en-us/library/system.net.webexception.status.aspx) of a WebException. So I guess, what I am trying to say is: If you want to reuse the features of an exception class, by all means, derive from it. If you don't want to reuse the features but just think that the name matches the category of your exception, you probably shouldn't derive from that class... – Daniel Hilgarth Sep 23 '13 at 15:35
  • @DanielHilgarth of course, however, the likelihood of their being re-usable features in those types of exceptions are *probably* quite low - and most likely easy enough to replicate. There is also the concern that you are becoming dependant on Framework architecture there because those particular properties you are leveraging could easily be changed/removed in future releases and as a result introduce breaking changes. – James Sep 24 '13 at 10:45
  • @James: You are dependent on the framework anyhow, because that's what you use throughout your application. Breaking changes in long established areas of the framework like the ones we are talking about are very unlikely. – Daniel Hilgarth Sep 24 '13 at 10:59
  • @DanielHilgarth I was meaning more from a "I expect X class to have Y property or Z functionality" - point being I would rather have complete control over my code than run the risk of having to patch my application later. Everyone's different, I guess it comes down to what you are going to gain by deriving - personally I can't see much. – James Sep 24 '13 at 11:16
  • 1
    @DanielHilgarth - he's a good [example](http://stackoverflow.com/questions/19378920/system-exception-hresult-is-inaccessible-due-to-its-protection-level/19378951#19378951) of the type of thing I was talking about....and on `Exception` of all classes! – James Oct 15 '13 at 12:13
  • 1
    @James: It was protected in *older* versions and now no longer is. That certainly isn't a breaking change. – Daniel Hilgarth Oct 15 '13 at 12:17
  • 1
    @DanielHilgarth from the OP's perspective it was, point is there was an unexpected change in the Framework which broke their app. Not looking to re-dig this debate back up just coincidental that this problem came up :) – James Oct 15 '13 at 12:22
  • 1
    @James: Really, that is absurd. In a new version of the .NET framework something is now there that wasn't there in an older version. And when he tries to run his program on an *older* version of the framework using the *new* feature, it logically doesn't work. I don't see how this is related at all. – Daniel Hilgarth Oct 15 '13 at 12:24
  • 1
    @DanielHilgarth I hardly think it's "absurd"...it's not a *new* feature though is it? It's a *change* to an existing property so yet again you miss the point. It's not even about using old .NET vs new .NET, it's about the fact that relying on the Framework **can lead to breaking changes** which is why I believe it's safer, if applicable, to keep as much code as you can in your control. This is just one example of *many* out there. – James Oct 15 '13 at 12:31
  • @James: I have the feeling you are suffering from the [NIH syndrome](http://en.wikipedia.org/wiki/Not_invented_here). Other than that, I can't say anymore, as we simply seem to disagree, and I *really* don't want to continue this fruitless discussion. – Daniel Hilgarth Oct 15 '13 at 12:35
  • 1
    @DanielHilgarth haha hardly, also it's not exactly a professional attitude to throw the toys out the pram because someone has an opposite view. I never once said either of our opinions were wrong, I just think it's sad that you seem to be too stubborn to actually see value in my point. Regardless, this "fruitless" conversation can end here. – James Oct 15 '13 at 12:42
-1

Since InvalidOperationexception derives from System.Exception, You would be fine to extend from it and add your own Message() method (C#'s version of multi-inheritance). Of course you will still have to update your throwing code throw your new Exception. But everywhere that already catches the InvalidOperationexception should not have to be touched.

Greg Jones
  • 399
  • 1
  • 7