1

What's better between several ChildException catch blocks and one Exception catch block? By better, I mean in a good-practices way.

To illustrate:

public static void main(String[] args) {
    System.out.println(Main.isNonsense1(null)); // false <- bad
    System.out.println(Main.isNonsense2(null)); // NullPointerException <- good
}

// More readable, less precise
public static boolean isNonsense1(String className) {
    try {
        Class.forName(className);
        String.class.getConstructor(String.class);
        className.getBytes("UTF-8");
        MessageDigest.getInstance("SHA-1").wait();
        return true;
    } catch (Exception e) {
        return false;
    }
}

// Less readable, more precise
public static boolean isNonsense2(String className) {
    try {
        Class.forName(className);
        String.class.getConstructor(String.class);
        className.getBytes("UTF-8");
        MessageDigest.getInstance("SHA-1").wait();
        return true;
    } catch (ClassNotFoundException e) {
        return false;
    } catch (NoSuchMethodException e) {
        return false;
    } catch (SecurityException e) {
        return false;
    } catch (UnsupportedEncodingException e) {
        return false;
    } catch (NoSuchAlgorithmException e) {
        return false;
    } catch (InterruptedException e) {
        return false;
    }
}
Luke
  • 3,742
  • 4
  • 31
  • 50
sp00m
  • 47,968
  • 31
  • 142
  • 252
  • If all you want is to return false, then dont't trouble yourself to specialize the cases. – ron May 22 '12 at 12:41

3 Answers3

4

This is related to this question: Catch multiple exceptions at once?

The answer there is good. The key is that if you catch Exception then you should handle each of the cases that you are aware of and throw all the rest. That is, simply catching Exception in your example and returning false would not be a good idea. You may inadvertently catch some exception you didn't mean to.

Using your example, here is my suggested code:

public static boolean isNonsense2(String className) {
    try {
        Class.forName(className);
        String.class.getConstructor(String.class);
        className.getBytes("UTF-8");
        MessageDigest.getInstance("SHA-1").wait();
        return true;
    } catch (Exception e) {
        if (e instanceof ClassNotFoundException
                || e instanceof NoSuchMethodException
                || e instanceof SecurityException
                || e instanceof UnsupportedEncodingException
                || e instanceof NoSuchAlgorithmException
                || e instanceof InterruptedException) {
            return false;
        } else {
            throw e;
        }
    }
}
Community
  • 1
  • 1
Luke
  • 3,742
  • 4
  • 31
  • 50
  • Thanks, nice idea to use `instanceof` by the way `:)` – sp00m May 22 '12 at 16:01
  • I should also warn that one downside of doing it this way is that the compiler will no longer warn you if you are handling a **checked exception** that cannot be thrown by the code in the `try` block. – Luke May 22 '12 at 16:41
1

I think there is no complete clear answer. In your case I would code it like this:

public static boolean isNonsense1(String className) {
    if(slassname==null) throw new IllegalArgumentException("className must not be null");
    try {
        Class.forName(className);
        String.class.getConstructor(String.class);
        className.getBytes("UTF-8");
        MessageDigest.getInstance("SHA-1").wait();
        return true;
    } catch (ClassNotFoundException e) {
        throw new IllegalArgumentException("provided class " + className + " not found");
    } catch (Exception e) {
        return false;
    }
}

For my flavor, throwing a NullPointerException is always bad, thats why I throw the IllegalArgumentException

Christian Kuetbach
  • 15,850
  • 5
  • 43
  • 79
0

If you are not interested in handling the exception (which you should as per best practices) don't bother with the explicit catches. The whole point of being able to handle specific exceptions is to enable you to handle them correctly.

Selim
  • 1,013
  • 9
  • 15