Method agentmain
has 36 lines of code (exceeds 25 allowed). Consider refactoring. Open
@SuppressFBWarnings("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION")
public static void agentmain(@Nonnull String agentArgument) throws Exception {
logger.info(format("%s: attached with agent argument: %s.%n", DcmAgent.class.getName(), agentArgument));
agentArgument = agentArgument.trim();
- Create a ticketCreate a ticket
Method agentmain
has a Cognitive Complexity of 12 (exceeds 5 allowed). Consider refactoring. Open
@SuppressFBWarnings("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION")
public static void agentmain(@Nonnull String agentArgument) throws Exception {
logger.info(format("%s: attached with agent argument: %s.%n", DcmAgent.class.getName(), agentArgument));
agentArgument = agentArgument.trim();
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method convertStringArray2Arguments
has a Cognitive Complexity of 11 (exceeds 5 allowed). Consider refactoring. Open
private static Object[] convertStringArray2Arguments(String action, String[] arguments, Class<?>[] parameterTypes) {
if (arguments.length < parameterTypes.length) {
final String message = format("Action %s need more argument! arguments: %s", action, Arrays.toString(arguments));
throw new IllegalStateException(message);
}
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method convertStringArray2Arguments
has 29 lines of code (exceeds 25 allowed). Consider refactoring. Open
private static Object[] convertStringArray2Arguments(String action, String[] arguments, Class<?>[] parameterTypes) {
if (arguments.length < parameterTypes.length) {
final String message = format("Action %s need more argument! arguments: %s", action, Arrays.toString(arguments));
throw new IllegalStateException(message);
}
- Create a ticketCreate a ticket
Method doAction
has 26 lines of code (exceeds 25 allowed). Consider refactoring. Open
private static boolean doAction(final String action, final List<String> arguments, final PrintWriter filePrinter) {
final String argumentString = join(arguments);
if (!action2Method.containsKey(action)) {
logger.info(format(("%s: Unknown action %s, ignore! action: %<s %s!%n"), DcmAgent.class.getName(), action, argumentString));
- Create a ticketCreate a ticket
Method doAction
has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring. Open
private static boolean doAction(final String action, final List<String> arguments, final PrintWriter filePrinter) {
final String argumentString = join(arguments);
if (!action2Method.containsKey(action)) {
logger.info(format(("%s: Unknown action %s, ignore! action: %<s %s!%n"), DcmAgent.class.getName(), action, argumentString));
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method parseAgentArgument
has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring. Open
@Nonnull
private static Map<String, List<String>> parseAgentArgument(@Nonnull String argument) {
final String[] split = argument.split("\\s+");
int idx = 0;
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method printActionResult
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring. Wontfix
private static void printActionResult(String action, Object result, PrintWriter writer) {
if (writer == null) return;
final Method method = action2Method.get(action);
if (method.getReturnType() == void.class) return;
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Define and throw a dedicated exception instead of using a generic one. Open
throw new RuntimeException("fail to getActionList, cause: " + e, e);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Using such generic exceptions as Error
, RuntimeException
, Throwable
, and Exception
prevents
calling methods from handling true, system-generated exceptions differently than application-generated errors.
Noncompliant Code Example
public void foo(String bar) throws Throwable { // Noncompliant throw new RuntimeException("My Message"); // Noncompliant }
Compliant Solution
public void foo(String bar) { throw new MyOwnRuntimeException("My Message"); }
Exceptions
Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the analysis).
@Override public void myMethod() throws Exception {...}
Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.
public void myOtherMethod throws Exception { doTheThing(); // this method throws Exception }
See
- MITRE, CWE-397 - Declaration of Throws for Generic Exception
- CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable
Add a private constructor to hide the implicit public one. Open
public class DcmAgent {
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Utility classes, which are collections of static
members, are not meant to be instantiated. Even abstract utility classes, which can
be extended, should not have public constructors.
Java adds an implicit public constructor to every class which does not define at least one explicitly. Hence, at least one non-public constructor should be defined.
Noncompliant Code Example
class StringUtils { // Noncompliant public static String concatenate(String s1, String s2) { return s1 + s2; } }
Compliant Solution
class StringUtils { // Compliant private StringUtils() { throw new IllegalStateException("Utility class"); } public static String concatenate(String s1, String s2) { return s1 + s2; } }
Exceptions
When class contains public static void main(String[] args)
method it is not considered as utility class and will be ignored by this
rule.
Rename this variable to not match a restricted identifier. Open
String var = System.getenv(name);
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Even if it is technically possible, Restricted Identifiers should not be used as identifiers. This is only possible for compatibility reasons, using it in Java code is confusing and should be avoided.
Note that this applies to any version of Java, including the one where these identifiers are not yet restricted, to avoid future confusion.
This rule reports an issue when restricted identifiers:
- var
- yield
- record
are used as identifiers.
Noncompliant Code Example
var var = "var"; // Noncompliant: compiles but this code is confusing var = "what is this?"; int yield(int i) { // Noncompliant return switch (i) { case 1: yield(0); // This is a yield from switch expression, not a recursive call. default: yield(i-1); }; } String record = "record"; // Noncompliant
Compliant Solution
var myVariable = "var"; int minusOne(int i) { return switch (i) { case 1: yield(0); default: yield(i-1); }; } String myRecord = "record";
See
Define and throw a dedicated exception instead of using a generic one. Open
public static void agentmain(@Nonnull String agentArgument) throws Exception {
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Using such generic exceptions as Error
, RuntimeException
, Throwable
, and Exception
prevents
calling methods from handling true, system-generated exceptions differently than application-generated errors.
Noncompliant Code Example
public void foo(String bar) throws Throwable { // Noncompliant throw new RuntimeException("My Message"); // Noncompliant }
Compliant Solution
public void foo(String bar) { throw new MyOwnRuntimeException("My Message"); }
Exceptions
Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the analysis).
@Override public void myMethod() throws Exception {...}
Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.
public void myOtherMethod throws Exception { doTheThing(); // this method throws Exception }
See
- MITRE, CWE-397 - Declaration of Throws for Generic Exception
- CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable
Invoke method(s) only conditionally. Open
logger.info(format("%s: attached with agent argument: %s.%n", DcmAgent.class.getName(), agentArgument));
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Passing message arguments that require further evaluation into a Guava com.google.common.base.Preconditions
check can result in a
performance penalty. That's because whether or not they're needed, each argument must be resolved before the method is actually called.
Similarly, passing concatenated strings into a logging method can also incur a needless performance hit because the concatenation will be performed every time the method is called, whether or not the log level is low enough to show the message.
Instead, you should structure your code to pass static or pre-computed values into Preconditions
conditions check and logging
calls.
Specifically, the built-in string formatting should be used instead of string concatenation, and if the message is the result of a method call,
then Preconditions
should be skipped altogether, and the relevant exception should be conditionally thrown instead.
Noncompliant Code Example
logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages logger.fine("An exception occurred with message: " + message); // Noncompliant LOG.error("Unable to open file " + csvPath, e); // Noncompliant Preconditions.checkState(a > 0, "Arg must be positive, but got " + a); // Noncompliant. String concatenation performed even when a > 0 Preconditions.checkState(condition, formatMessage()); // Noncompliant. formatMessage() invoked regardless of condition Preconditions.checkState(condition, "message: %s", formatMessage()); // Noncompliant
Compliant Solution
logger.log(Level.SEVERE, "Something went wrong: {0} ", message); // String formatting only applied if needed logger.fine("An exception occurred with message: {}", message); // SLF4J, Log4j logger.log(Level.SEVERE, () -> "Something went wrong: " + message); // since Java 8, we can use Supplier , which will be evaluated lazily LOG.error("Unable to open file {0}", csvPath, e); if (LOG.isDebugEnabled() { LOG.debug("Unable to open file " + csvPath, e); // this is compliant, because it will not evaluate if log level is above debug. } Preconditions.checkState(arg > 0, "Arg must be positive, but got %d", a); // String formatting only applied if needed if (!condition) { throw new IllegalStateException(formatMessage()); // formatMessage() only invoked conditionally } if (!condition) { throw new IllegalStateException("message: " + formatMessage()); }
Exceptions
catch
blocks are ignored, because the performance penalty is unimportant on exceptional paths (catch block should not be a part of
standard program flow). Getters are ignored as well as methods called on annotations which can be considered as getters. This rule accounts for
explicit test-level testing with SLF4J methods isXXXEnabled
and ignores the bodies of such if
statements.
Invoke method(s) only conditionally. Open
logger.info(format(("%s: Unknown action %s, ignore! action: %<s %s!%n"), DcmAgent.class.getName(), action, argumentString));
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Passing message arguments that require further evaluation into a Guava com.google.common.base.Preconditions
check can result in a
performance penalty. That's because whether or not they're needed, each argument must be resolved before the method is actually called.
Similarly, passing concatenated strings into a logging method can also incur a needless performance hit because the concatenation will be performed every time the method is called, whether or not the log level is low enough to show the message.
Instead, you should structure your code to pass static or pre-computed values into Preconditions
conditions check and logging
calls.
Specifically, the built-in string formatting should be used instead of string concatenation, and if the message is the result of a method call,
then Preconditions
should be skipped altogether, and the relevant exception should be conditionally thrown instead.
Noncompliant Code Example
logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages logger.fine("An exception occurred with message: " + message); // Noncompliant LOG.error("Unable to open file " + csvPath, e); // Noncompliant Preconditions.checkState(a > 0, "Arg must be positive, but got " + a); // Noncompliant. String concatenation performed even when a > 0 Preconditions.checkState(condition, formatMessage()); // Noncompliant. formatMessage() invoked regardless of condition Preconditions.checkState(condition, "message: %s", formatMessage()); // Noncompliant
Compliant Solution
logger.log(Level.SEVERE, "Something went wrong: {0} ", message); // String formatting only applied if needed logger.fine("An exception occurred with message: {}", message); // SLF4J, Log4j logger.log(Level.SEVERE, () -> "Something went wrong: " + message); // since Java 8, we can use Supplier , which will be evaluated lazily LOG.error("Unable to open file {0}", csvPath, e); if (LOG.isDebugEnabled() { LOG.debug("Unable to open file " + csvPath, e); // this is compliant, because it will not evaluate if log level is above debug. } Preconditions.checkState(arg > 0, "Arg must be positive, but got %d", a); // String formatting only applied if needed if (!condition) { throw new IllegalStateException(formatMessage()); // formatMessage() only invoked conditionally } if (!condition) { throw new IllegalStateException("message: " + formatMessage()); }
Exceptions
catch
blocks are ignored, because the performance penalty is unimportant on exceptional paths (catch block should not be a part of
standard program flow). Getters are ignored as well as methods called on annotations which can be considered as getters. This rule accounts for
explicit test-level testing with SLF4J methods isXXXEnabled
and ignores the bodies of such if
statements.