debugjavaMinor
Reducing cyclomatic complexity
Viewed 0 times
complexityreducingcyclomatic
Problem
I ran a sonar analysis on my code and it told me the cyclomatic complexity is too high (sonar's limit is ten branches).
Here is my code:
Basically, I want to catch a large set of Exceptions, (maybe always processing the same behaviour). I read the question Catching multiple types of exceptions when writing JSON but I don't use Java 7 (with which I could have all Exceptions in one catch statement) and I do not really want to catch ALL Exceptions, since I want my code to fail in a case I did not expect.
Is there any alternative that would involve fewer branches?
NOTE: What I want to achieve here is not to recover from any Exception, but to categorize the Exceptions. An upper layer is in charge of handling Exceptions, but for this purpose, Exceptions need to be properly categorized.
Here is my code:
public void myMethod(){
try{
// do something
catch(SomeException e){
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
catch(AnotherException e){
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
// lots of other Exceptions
}Basically, I want to catch a large set of Exceptions, (maybe always processing the same behaviour). I read the question Catching multiple types of exceptions when writing JSON but I don't use Java 7 (with which I could have all Exceptions in one catch statement) and I do not really want to catch ALL Exceptions, since I want my code to fail in a case I did not expect.
Is there any alternative that would involve fewer branches?
NOTE: What I want to achieve here is not to recover from any Exception, but to categorize the Exceptions. An upper layer is in charge of handling Exceptions, but for this purpose, Exceptions need to be properly categorized.
Solution
As JohnMark13 stated in the comments above, if the exceptions you want to catch share a common supertype (possibly besides the basic
Also, I think that since you rethrow the exceptions (or at least most of them), then catching the general
Perhaps you get some extra cyclomatic complexity from the
Depending on the specific
Exception class), then you could catch the supertype instead of all the subtypes. If you have created the SomeException and AnotherException classes yourself, then I strongly suggest that you make them extend a common custom exception type.Also, I think that since you rethrow the exceptions (or at least most of them), then catching the general
Exception is OK. Or, you could first catch all RuntimeExceptions, rethrow them directly, and then check the rest and possibly throw your WrappingException. Consider this:public void myMethod() {
try {
// do something
}
catch (RuntimeException e) {
// We don't need to wrap these, so we just throw the same exception again
throw e;
}
catch (YourCommonSuperException e) {
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
catch (Exception e) {
// All other excpetions, such as `IOException` and stuff will be caught and wrapped and throwed here.
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
// No need for any other Exceptions
}Perhaps you get some extra cyclomatic complexity from the
// do something part. If so, then you could split that part into multiple methods and possibly declaring to throw the exception from the method if needed so that it will get caught in this myMethod() (or catch within the method and throw some kind of YourCommonSuperException.Depending on the specific
// do something part, you might want to overlook which kind of exceptions that are thrown there. If you throw them yourself, it might be possible to change to IllegalArgumentException, IllegalStateException (which both extend RuntimeException) or some exception that extends YourCommonSuperException.Code Snippets
public void myMethod() {
try {
// do something
}
catch (RuntimeException e) {
// We don't need to wrap these, so we just throw the same exception again
throw e;
}
catch (YourCommonSuperException e) {
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
catch (Exception e) {
// All other excpetions, such as `IOException` and stuff will be caught and wrapped and throwed here.
throw new WrappingException("issue when doing something " + e.getMessage(), e);
}
// No need for any other Exceptions
}Context
StackExchange Code Review Q#31477, answer score: 2
Revisions (0)
No revisions yet.