views:

186

answers:

8

At the company where I am right now, there are a lot of places in the code where an OurCompanyRuntimeException is thrown (where OurCompany is the actual name of the company). As far as I can tell, this exception is described as "A runtime exception thrown by code that we wrote here at this company."

I'm somewhat new to Java, but I thought exception types were supposed to reflect what went wrong, not whose code threw the exception. For example, IllegalArgumentException means someone passed an illegal argument to something. You wouldn't have a SunIllegalArgumentException if an illegal argument was passed in code written by Sun, and then a IBMIllegalArgumentException -- that would be silly and pointless, right? And if you want to know where the exception was thrown, you look at the stack trace. I understand wanting to extend RuntimeException (so that you don't have as many try/catches or "throws"es all over) but why not make subclasses that explain what happened, as opposed to what company's code it happened in?

Has anyone ever used the OurCompanyRuntimeException idea before, or have an idea of why they might have done it this way?

+2  A: 

Helps when reading stack traces, that's for sure. I.e. when scanning through a lot of lines of 'caused by's', it helps to see that it occurred in something thrown by you, not something internal to, say, a container.

Also lets you perform custom actions as part of the throwable - e.g. write to a special log somewhere, etc.

Chaos
+6  A: 

Sounds like the usual bespoke code nonsense that you find in in-house code bases to me. I suspect if you ask about there'll have been some decree that the OurCompanyRuntimeException was used based on some misguided logic by a senior guy that nobody dared question and has long since moved on - the story of the monkeys, bananas and hose-pipes springs to mind.

I agree with you, the name of the exception should be indicative of the error that has occurred.

Nick Holt
+1  A: 

This is a bad concept. Exceptions should be specific for a use case.

Okay, if the company does produce a lot of faulty code/products, they may use that type of exception as advertisement ;)

oeogijjowefi
+1 for the perverse idea of advertising through log statements.
Steve B.
+2  A: 

Yes I encountered that, too, but it didn't make sense to me either. My guess was, that the companies wrote this exceptions very early after the adoption of Java without getting the correct idea of how exception throwing and handling really works (like Nick already said... by the senior programmer nobody dared to question). If the company feels the need to create its own exception class (e.g. for company specific logging porpuses) this exception should never be thrown directly (making it abstract). I would derive concrete Problem describing Exceptions instead or just follow the Spring Framework's idea for Exception handling/throw.

Daff
+1  A: 

Your company might be adding code to an already existing project, like an open source code base, and could have just added very little code to it. So in order to trace errors that happen by the company's developers, they thought that they would have their own exception class to distinguish the errors that were there before, from the ones caused by the extension. This way, they can focus only on the ones that are caused by company's developers, and perhaps ask the original source code maintainers to fix the other ones.

With time, when you people have developed a sufficiently large code base through in-house development, you may add more exceptions and remove the CompanynameRuntimeException altogother. Also, they might get more at ease with the level of expertise of the developers, to allow them to treat all errors like one, and not to view the ones caused by company developers more suspiciously.

+1  A: 

It would make very good sense to have this as a baseclass for specific exceptions. You thrown a specific exception and catch the base class.

Also it may allow to carry a cause (the REAL exception) plus additional information around. That can be very handy for creating diagnostic output for logging.

Thorbjørn Ravn Andersen
I checked the hierarchy, and OurCompanyRuntimeException has 20-some subclasses like DeleteException, OurCompanySQLException, and DatabaseTransactionException. So it may be that this was the original intention, and they just didn't think to enforce it by making the base class abstract. Most OurCompanyRuntimeExceptions I've seen do indeed carry a cause, and that does make log messages much more helpful.
MatrixFrog
Talk to your boss. It might be an afternoon excercise making the base class abstract and fixing all the resulting compilation errors.
Thorbjørn Ravn Andersen
+1  A: 

Seems pretty silly, logging output or a stack trace will show you who the offending class is, so that explanation doesn't wash. Also seems dangerous, as if people are encouraged to throw the OurCompanyRuntimeException they're throwing RuntimeExceptions, which don't force the caller to handle them and can take down your application.

I agree with you that exceptions should reflect the reason behind them. I've seen a custom exception as the root of a hierarchy, although it probably ought to be abstract, so that you need to create a specific extension to use one, and it definitely shouldn't be a RuntimeException.

Steve B.
I think there may have been both an `OurCompanyRuntimeException` and also an `OurCompanyException`
MatrixFrog
A: 

It wouldn't be a bad idea to have a generic company wide exception class like you describe that more specific exception cases inherit from. One answer already mentioned the ability specifically catch internal code exceptions and ignore / pass through the ones from core java or third party library code. The key point here is that more specific exceptions should inherit from this one. Throwing a generic company-named exception would be rarely required, and almost never recommended.

Chris