HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

Use of Exception to log execution stack trace

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stackexceptionlogusetraceexecution

Problem

I've been back and forth with a colleague over the use of Throwable.fillInStackTrace. This Log class is meant to wrap the Simple Logging Facade for Java.

public class Log {
    private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(Log.class);

    private String stackTrace(Exception cause) {
        if (cause == null)
            return "";
        StringWriter sw = new StringWriter(1024);
        final PrintWriter pw = new PrintWriter(sw);
        cause.printStackTrace(pw);
        pw.flush();
        return sw.toString();
    }

    public void warn(Exception cause, String message) {
        String output = message + "\n" + stackTrace(cause);
        log.warn(output);
    }

    // debug(...), info(...), etc.
}


But it is being used thusly:

new Log()
    .warn((Exception)new RuntimeException().fillInStackTrace(), "I'm warning you.");


When reviewing this, my comments included things like the "Exceptions are exceptional" idiom.

But when I mentioned that creating a stack trace is an expensive operation and should be avoided, I was told that this used to be true in the past [e.g. with JVM implementations] but is now longer true [i.e. This has been fixed/optimized].

I was not aware of this development, so I checked around and most of the SO questions seem to indicate that making a stack trace is expensive.

Does this code seem reasonable? Would you consider this bad practice?

Solution

It seems like you're working harder than you need to. Instead of this:

public void warn(Exception cause, String message) {
    String output = message + "\n" + stackTrace(cause);
    log.warn(output);
}


I think you could do this, and no need for the stackTrace method:

public void warn(Exception cause, String message) {
    log.error(cause.getMessage() + " " + message, cause);
}


Moreover, notice how common libraries use a String as the first parameter of logging methods, and a Throwable as the second. I suggest to follow that example and reorder your parameters.

Code Snippets

public void warn(Exception cause, String message) {
    String output = message + "\n" + stackTrace(cause);
    log.warn(output);
}
public void warn(Exception cause, String message) {
    log.error(cause.getMessage() + " " + message, cause);
}

Context

StackExchange Code Review Q#68233, answer score: 8

Revisions (0)

No revisions yet.