Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I am tring to count the number of exceptions happening and log those exceptions as well. So what I did is, I created one method addException in which I am counting all the exceptions.

addException method accepts two parameters, one is the String, and other is the boolean flag which means whether we want to terminate the program or not because of any exceptions. Meaning, if that flag is true, then I need to terminate the program whenever there are any exceptions.

So if you take a look into my below catch block, I have addException method call for counting the exceptions and below that method call I am logging the exceptions as well.

catch (ClassNotFoundException e) {
    addException(e.getCause() != null ? e.getCause().toString() : e.toString(), Read.flagTerminate);
    LOG.error("Threw a ClassNotFoundException in " + getClass().getSimpleName(), e);
} catch (SQLException e) {
    addException(e.getCause() != null ? e.getCause().toString() : e.toString(), Read.flagTerminate);
    //DAMN! I'm not....
    LOG.error("Threw a SQLException while making connection to database in " + getClass().getSimpleName(), e);
}


/**
 * A simple method that will add the count of exceptions and name of
 * exception to a map
 * 
 * @param cause
 * @param flagTerminate 
 */
private static void addException(String cause, boolean flagTerminate) {
    AtomicInteger count = exceptionMap.get(cause);
    if (count == null) {
        count = new AtomicInteger();
        AtomicInteger curCount = exceptionMap.putIfAbsent(cause, count);
        if (curCount != null) {
            count = curCount;
        }
    }
    count.incrementAndGet();

    if(flagTerminate) {
        System.exit(1);
    }
}

Problem Statement:-

Now what I am looking for is-

Is there any more cleaner way of doing the same thing? Meaning right now I am counting the exceptions in a method and then printing out the exceptions in the next line inside the catch block.

Is it possible to do the both of the things in the same addException method? And if the flag is true to terminate the program, then terminate the program with the proper logging as well.

What could be the best way to re write addException method do this?

share|improve this question

1 Answer

up vote 1 down vote

Is there any more cleaner way of doing the same thing? Meaning right now I am counting the exceptions in a method and then printing out the exceptions in the next line inside the catch block. Is it possible to do the both of the things in the same addException method?

It depends. You could configure the logger and add a new a appender. Then your logger serves two appenders: The (probably) println and the counting.
Otherwise, you could add the log call inside the addException method (could be renamed to handleException(...) then).

And if the flag is true to terminate the program, then terminate the program with the proper logging as well.

I highly suggest to avoid boolean flag arguments. If you have a public api, this suggestion is a must. Look at java for a lot of bad examples.
Use two methods:

private static void addException(String cause) {
  ...
}

private static void addExceptionAndTerminate(String cause) {
  addException(...);
  System.exit(1);
}

(And I do not see the point of adding the exception if you terminate anyway, but it could be for some multithreading things.)

What could be the best way to re write addException method do this?

I would change the method signature from String, Boolean to Exception and put all exceptions inside a synchronized list.
Only if I want to get some statistics, I would start to parse them. Which makes the live easier for everyone who is using this.
At the moment, the functionality depends on the format of the string. If you or someone else, does not use the correct format, it does not work. therefore, you sould document this at least in the javadoc. And the correct formatting is a pain, noone will like to write like this.
And I am not sure if the implementation for concurrent map access is correct.

If you just allow an exception object, noone has to care about formatting, the synchronization is a lot easier (just add to the list) and you have all the freedom in some other method how to count, parse and do whatever you want with this exceptions.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.