CODEWITHSUNDEEP
java
lindelof
lindelof

Reputation: 35240

Is it ok to instantiate an exception without throwing it?

Suppose I have a MyException class that subclasses Exception. I'm using this class to include contextual information when errors occur in my code.

I typically use it to wrap one of the "standard" exception classes. For example, if an error happens during input validation, I will do something like

if (invalidInput())
  throw new MyException(new IllegalArgumentException(), arg1, arg2, ...);

But my IDE (Intellij IDEA) warns me that instantiating an unchecked exception (IllegalArgumentException in this example) without throwing it is bad, but doesn't tell me why.

So how sinful is it to instantiate an exception without throwing it? To which circle of hell will I go?

Upvotes: 7

Views: 2940

Answers (5)

Peter Lawrey
Peter Lawrey

Reputation: 533492

IntelliJ does tell you why. You just have to read the description for the inspection.

This inspection reports any instances of Throwable instantiation, where the created Throwable is never actually thrown. Most often this is the result of a simple mistake.

Upvotes: 1

user85421
user85421

Reputation: 29680

It's a strange construct, but there is a solution to trick the compiler.
Rewrite your invalidInput method to throw the IAE 

    private void checkInput() throws IllegalArgumentException {
        if (...)
            throw new IllegalArgumentException();

and: 

    try {
        checkInput();
    } catch (IllegalArgumentException ex) {
        throw new MyException(ex, arg1, arg2, ...);
    }

Upvotes: 2

Joel
Joel

Reputation: 30146

You might be better off throwing an instance of IllegalArgumentException, in this case that's what it's for:

if (invalidInput())
         new IllegalArgumentException("Invalid argument " + x + ", expected ...");

Or otherwise extending, IllegalArgumentException instead of Exception, it if you want to enhance it with custom properties.

public class MyIllegalArgumentException extends IllegalArgumentException {  
    public MyIllegalArgumentException(Object arg...) {    ....  } 
}

Both cases provide a leaner, more meaningful, class model.

Update: given your comment about wanting to supply contextual info with the thrown exception - you can do this by supplying your custom exception object as the Throwable argument to the standard exceptions contructor i.e. flip it round so: instead of wrapping the relevant standard exception in your exception, you should wrap your exception in the relevant standard exception.

if (invalidInput())
         new IllegalArgumentException("Invalid argument " + x + ", expected ...", new MyContextException(a,b,c));

(where a,b & c are the various bits of context you want to transmit). This way you (re)use a meaningful & appropriate, exception at all points in the code, but you transmit the contextual information that you may want to use further up the stack when handling/logging the exception.

Upvotes: 8

Andrzej Doyle
Andrzej Doyle

Reputation: 103787

This isn't bad at all.

Like most warnings, they're there to indicate situations that are less likely to legitimately occur, than they are to be someone invoking them by mistake. Another recent example on SO was the warnings about synchronizing on a local variable; it's not often you want to do that and it's easy to mess up concurrency by doing it inadvertently.

IntelliJ is just warning you that exceptions are usually created to be thrown immediately, so if you aren't doing this it flags that perhaps your code is behaving unexpectedly. In your case, you're doing exactly what's right so feel free to ignore the warning. (In fact in my project the structure means I often create exceptions without throwing them for various reasons, so I downgraded the warning to an "info" level in IntelliJ's config).

Upvotes: 2

Buhb
Buhb

Reputation: 7149

You're not going to hell from instantiating an exception the way you're doing it. I'm sure the warning is there to make sure you use your exceptions as exceptions and not just any object.

However, you're going to hell for having MyException not subclassing RuntimeException ;)

Upvotes: 2

Related Questions