CODEWITHSUNDEEP
javaexception
Alexander Mills
Alexander Mills

Reputation: 100010

Creating my own Java Exception subclass

I am creating my own Exception class for a simple command line tool I created. Basically the purpose is to decide when a fatal Exception is thrown and to terminate the program in that case. So my ReconToolException class has a boolean flag to record a fatal Exception.

However, my question is, what is the proper way to "throw" the Exception?

Should I do it this way?:

if(columnNumberForGroupByColumnFile1 == null || columnNumberForGroupByColumnFile2 == null ||
            columnNumberForCountColumnFile1 == null || columnNumberForCountColumnFile2 == null ){

              new ReconToolException("Can't find column number. Likely that there are unexpected headers in the CSV files. Check your CSV configuration enum .java files to see if they match the actual input CSV files.");

    }

or should I do it the following way?:

if(columnNumberForGroupByColumnFile1 == null || columnNumberForGroupByColumnFile2 == null ||
                columnNumberForCountColumnFile1 == null || columnNumberForCountColumnFile2 == null ){
             try {
                 throw new ReconToolException("Can't find column number. Likely that there are unexpected headers in the CSV files. Check your CSV configuration enum .java files to see if they match the actual input CSV files.");
            } catch (ReconToolException e) {
                e.printStackTrace();
            }
        }

the former seems to require that I explicitly code a print out statement in my ReconToolException class to print the error to the command line. The latter will catch itself (which is very strange AND honestly seems incorrect) and will print out the stack trace in the catch block. The former seems to make more sense, but the latter seems to work better. Is there a correct way?

here is the Exception class I created:

public class ReconToolException extends Exception {

    private boolean isFatal;

    public ReconToolException() {
        // TODO Auto-generated constructor stub
    }
    public ReconToolException(boolean isFatalException, String msg) {
        super(msg);
        this.setFatal(isFatalException);
    }

    public ReconToolException(String message) {
        super(message);
        // TODO Auto-generated constructor stub
    }

    public ReconToolException(Throwable cause) {
        super(cause);
        // TODO Auto-generated constructor stub
    }

    public ReconToolException(String message, Throwable cause) {
        super(message, cause);
        // TODO Auto-generated constructor stub
    }

    public ReconToolException(String message, Throwable cause,
            boolean enableSuppression, boolean writableStackTrace) {
        super(message, cause, enableSuppression, writableStackTrace);
        // TODO Auto-generated constructor stub
    }


    public boolean isFatal() {
        return isFatal;
    }

    public void setFatal(boolean isFatal) {
        this.isFatal = isFatal;
    }

}

Upvotes: 3

Views: 190

Answers (3)

Maarten Bodewes
Maarten Bodewes

Reputation: 93968

To me this is a design mistake that is also a common one in languages that don't have exception handling. The problem is that the code that generates the exception gets to define what is fatal and what is not. However, if something is fatal or not depends on the calling code. Your isFatal field (which could simply be called fatal) might indicate that something is fatal when run in a CLI. But you could also be running it as a web-application that reads lines from a text field in HTML, and in that case the web-server should keep running.

It's probably better to encapsulate the meaning of the exception using field or by creating multiple subclasses of Exception. You can indicate how serious the problem is in the documentation. The determination of the severity is then up to the calling classes.

Personally I never use e.printStackTrace(); I even replace it with different code in code templates offered by my IDE:

// TODO create more specific exception handling
throw new RuntimeException("Unhandled exception", e);

If the calling code thinks that the exception thrown is fatal then it can throw a new RuntimeException or one of its subclasses with a clear description. Thus should then propagate all the way back to the start of execution of the module. If it passes the main method then the process will terminate after the stack trace is printed.

Upvotes: 1

Craig Otis
Craig Otis

Reputation: 32054

You should simply throw the exception, without catching it yourself in the method that's throwing it, unless you want to catch it there. Normally you don't, if you're creating the exception yourself. Other callers of this method may want to catch it though.

This will require that you modify your method signature to include throws ... indicating to callers what exceptions your method has the potential to throw:

if(columnNumberForGroupByColumnFile1 == null || columnNumberForGroupByColumnFile2 == null ||
                columnNumberForCountColumnFile1 == null || columnNumberForCountColumnFile2 == null ){

        throw new ReconToolException("Can't find column number. Likely that there are unexpected headers in the CSV files. Check your CSV configuration enum .java files to see if they match the actual input CSV files.");
     }
        ...

Upvotes: 4

M A
M A

Reputation: 72854

If you follow the second approach (i.e. catching the exception), then there is no meaning for the isFatal flag. If you simply throw and immediately catch the exception, then you are just displaying an error stack trace, and not allowing the exception to propagate up the sequence of method calls, and hence it cannot terminate the program in case it is fatal.

You should throw the exception, and let the caller of the method handle it.

Upvotes: 3

Related Questions