3

So, I was thinking about writing custom exceptions today, and considered the invalid operation exception. This exception can mean many many things, and in some actions, the operations might be invalid due to multiple independent causes. Consider this example. (And yes, I know that an ArgumentException would be better here, but I'm trying to come up with a simple example of an invalid operation exception)

public GetLocationFromLatLong(float Lat, float Long){
 if ( Lat < -90 || Lat > 90){
   throw new InvalidOperationException("Latitude is out of bounds")
 }
 if ( Long < -180 || Long > 180){
   throw new InvalidOperationException("Longitude is out of bounds")
 }

  /*Actual function here*/

}

If I wanted to handle the above exception, I would need to inspect the message to see which argument was out of range, and then prompt the user. While this isn't exactly BAD, the method could alternatively throw something like "InvalidLatitude" and "InvalidLongitude" exceptions (both of which would inherit from System.InvalidOperationException and be functionally identical), and I could catch either of those individually and take action from that. The question is, these exceptions would be structurally no different than System.InvalidOperationException, so I would be adding another layer of abstraction on to the code that may not be necessary.

I feel like this would make code more readable in some instances, but at the same time I've often heard that "Less is more" in terms of coding. Is this a code smell, or is there a reason to not do this?

  • 3
    Is there any circumstance in which the error handler will do something substantially different when handling a bad latitude vs. handling a bad longitude? If not, there is no reason to have them both. – Kilian Foth Jun 3 '16 at 5:06
  • Handling ArgumentException/InvalidOperationException is rarely a good idea. Normally those should only be thrown when you have a bug. – CodesInChaos Jun 3 '16 at 9:50
  • @CodesInChaos Imagine this function is called with user supplied data. While I would hope the validator in the UI layer would check this, it would have problems later on down the line assuming it didn't. My code can't assume calling code would supply correct values, so I either continue naively or I handle it as an exception. – Sidney Jun 3 '16 at 13:41
  • @KilianFoth For the above situation the only difference would be which error message would be displayed to the user, but it's not hard to image a situation in which there were different actions to be taken. – Sidney Jun 3 '16 at 13:42
2

You "only" need a separate exception type if you want to handle it separately.

IFF you want to catch a certain exception type separately from other errors, then creating a dedicated type for it (no matter from what it is derived) is the best option IMHO.

That being said, I think this is a rather big if for most cases.

Especially the InvalidOperationException - at least as I use it and understand it, is a case where I most definitely don't want to catch anything specific, because many times it indicates a bug in the code.

Aside: I just took a look at the code of my last project and the mostly the only things I catch explicitly there are FileNotFoundException and some other varieties of IOException and System.Data.Common.DbException - all other code does catch(Exception) and either rethrows with throw; or is at the very top of an operation stack.

1

I would need to inspect the message to see which argument was out of range, and then prompt the user. While this isn't exactly BAD

It's certainly a code smell from my PoV. Leaving aside whether you want to handle longitude and latitude errors differently, I would certainly consider one of the following options:

  1. a new subclass for each error. In this scenario it's not too bad, since you only have two obvious subclasses
  2. a flag or enumeration in your exception to indicate a longitude or latitude error. Again, you will need a subclass here, but only the one

Neither seem a particularly bad option to me. However, if you have to manage lots of these issues, I would perhaps consider other options, such as using a builder for a LatLong object. In that case, you could simply throw an InvalidOperationException (or InvalidArgument if you have such a thing - I'm not familiar with C#) and the method you're calling (SetLongitude()) would make the context and cause specific. In this scenario, perhaps overkill. In more complex scenarios, perhaps something to investigate

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.