Showing 651 of 651 total issues
Avoid reassigning parameters such as 'linePtr' Open
private int collectNodes(Session session, int linePtr) {- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
AvoidReassigningParameters
Since: PMD 1.0
Priority: Medium High
Categories: Style
Remediation Points: 50000
Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.
Example:
public class Foo {
private void foo(String bar) {
bar = 'something else';
}
}All methods are static. Consider using a utility class instead. Alternatively, you could add a private constructor or make the class abstract to silence this warning. Open
public class BitOperator {
/**
* Returns the bit in byte b at the given index. Note that index must be
* between 0-7- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UseUtilityClass
Since: PMD 0.3
Priority: Medium
Categories: Style
Remediation Points: 50000
For classes that only have static methods, consider making them utility classes. Note that this doesn't apply to abstract classes, since their subclasses may well include non-static methods. Also, if you want this class to be a utility class, remember to add a private constructor to prevent instantiation. (Note, that this use was known before PMD 5.1.0 as UseSingleton).
Example:
public class MaybeAUtility {
public static void foo() {}
public static void bar() {}
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}A switch with less than three branches is inefficient, use a if statement instead. Open
switch(nextHeaderType) {
default: return Protocol.get("END_PROTOCOL");
}- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
TooFewBranchesForASwitchStatement
Since: PMD 4.2
Priority: Medium
Categories: Style
Remediation Points: 50000
Switch statements are intended to be used to support complex branching behaviour. Using a switch for only a few cases is ill-advised, since switches are not as easy to understand as if-then statements. In these cases use the if-then statement to increase code readability.
Example:
// With a minimumNumberCaseForASwitch of 3
public class Foo {
public void bar() {
switch (condition) {
case ONE:
instruction;
break;
default:
break; // not enough for a 'switch' stmt, a simple 'if' stmt would have been more appropriate
}
}
}Consider simply returning the value vs storing it in local variable 'mav' Open
return mav;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}Position literals first in String comparisons for EqualsIgnoreCase Open
} else if (mode.equalsIgnoreCase("hex")) {- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
PositionLiteralsFirstInCaseInsensitiveComparisons
Since: PMD 5.1
Priority: Medium
Categories: Style
Remediation Points: 50000
Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false.
Example:
class Foo {
boolean bar(String x) {
return x.equalsIgnoreCase('2'); // should be '2'.equalsIgnoreCase(x)
}
}Avoid unnecessary if..then..else statements when returning booleans Open
if (target < lowerLimit || target > upperLimit) {
return false;
}- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
SimplifyBooleanReturns
Since: PMD 0.9
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid unnecessary if-then-else statements when returning a boolean. The result of the conditional test can be returned instead.
Example:
public boolean isBarEqualTo(int x) {
if (bar == x) { // this bit of code...
return true;
} else {
return false;
}
}
public boolean isBarEqualTo(int x) {
return bar == x; // can be replaced with this
}When doing a String.toLowerCase()/toUpperCase() call, use a Locale Open
cellTable.put(protocolName.toUpperCase(), cellStage);- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UseLocaleWithCaseConversions
Since: PMD 2.0
Priority: Medium
Categories: Style
Remediation Points: 50000
When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that have unusual conventions, i.e. Turkish.
Example:
class Foo {
// BAD
if (x.toLowerCase().equals('list')) { }
/*
* This will not match 'LIST' when in Turkish locale
* The above could be
* if (x.toLowerCase(Locale.US).equals('list')) { }
* or simply
* if (x.equalsIgnoreCase('list')) { }
*/
// GOOD
String z = a.toLowerCase(Locale.EN);
}Private field 'graphLines' could be made final; it is only initialized in the declaration or constructor. Open
private ArrayList<String> graphLines = new ArrayList<String>();- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
ImmutableField
Since: PMD 2.0
Priority: Medium
Categories: Style
Remediation Points: 50000
Identifies private fields whose values never change once they are initialized either in the declaration of the field or by a constructor. This helps in converting existing classes to becoming immutable ones.
Example:
public class Foo {
private int x; // could be final
public Foo() {
x = 7;
}
public void foo() {
int a = x + 2;
}
}Avoid if (x != y) ..; else ..; Open
if (!graphLines.get(linePtr).contains("graph")) {
linePtr++;
} else {
break;
}- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
ConfusingTernary
Since: PMD 1.9
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid negation within an 'if' expression with an 'else' clause. For example, rephrase: if (x != y) diff(); else same(); as: if (x == y) same(); else diff();. Most 'if (x != y)' cases without an 'else' are often return cases, so consistent use of this rule makes the code easier to read. Also, this resolves trivial ordering problems, such as 'does the error case go first?' or 'does the common case go first?'.
Example:
boolean bar(int x, int y) {
return (x != y) ? diff : same;
}Position literals first in String comparisons for EqualsIgnoreCase Open
if (mode.equalsIgnoreCase("ip4")) {- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
PositionLiteralsFirstInCaseInsensitiveComparisons
Since: PMD 5.1
Priority: Medium
Categories: Style
Remediation Points: 50000
Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false.
Example:
class Foo {
boolean bar(String x) {
return x.equalsIgnoreCase('2'); // should be '2'.equalsIgnoreCase(x)
}
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}Perhaps 'graphParser' could be replaced by a local variable. Open
private ProtocolGraphParser graphParser;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
SingularField
Since: PMD 3.1
Priority: Medium
Categories: Style
Remediation Points: 50000
Fields whose scopes are limited to just single methods do not rely on the containing object to provide them to other methods. They may be better implemented as local variables within those methods.
Example:
public class Foo {
private int x; // no reason to exist at the Foo instance level
public void foo(int y) {
x = y + 5;
return x;
}
}When doing a String.toLowerCase()/toUpperCase() call, use a Locale Open
return cellTable.get(protocolName.toUpperCase());- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UseLocaleWithCaseConversions
Since: PMD 2.0
Priority: Medium
Categories: Style
Remediation Points: 50000
When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that have unusual conventions, i.e. Turkish.
Example:
class Foo {
// BAD
if (x.toLowerCase().equals('list')) { }
/*
* This will not match 'LIST' when in Turkish locale
* The above could be
* if (x.toLowerCase(Locale.US).equals('list')) { }
* or simply
* if (x.equalsIgnoreCase('list')) { }
*/
// GOOD
String z = a.toLowerCase(Locale.EN);
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}When doing a String.toLowerCase()/toUpperCase() call, use a Locale Open
return classTable.get(protocolName.toUpperCase());- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UseLocaleWithCaseConversions
Since: PMD 2.0
Priority: Medium
Categories: Style
Remediation Points: 50000
When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that have unusual conventions, i.e. Turkish.
Example:
class Foo {
// BAD
if (x.toLowerCase().equals('list')) { }
/*
* This will not match 'LIST' when in Turkish locale
* The above could be
* if (x.toLowerCase(Locale.US).equals('list')) { }
* or simply
* if (x.equalsIgnoreCase('list')) { }
*/
// GOOD
String z = a.toLowerCase(Locale.EN);
}All methods are static. Consider using a utility class instead. Alternatively, you could add a private constructor or make the class abstract to silence this warning. Open
public class Security {
public static final String SHA256_ALGORITHM = "SHA-256";
public static String createHash(String str) {- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UseUtilityClass
Since: PMD 0.3
Priority: Medium
Categories: Style
Remediation Points: 50000
For classes that only have static methods, consider making them utility classes. Note that this doesn't apply to abstract classes, since their subclasses may well include non-static methods. Also, if you want this class to be a utility class, remember to add a private constructor to prevent instantiation. (Note, that this use was known before PMD 5.1.0 as UseSingleton).
Example:
public class MaybeAUtility {
public static void foo() {}
public static void bar() {}
}Consider simply returning the value vs storing it in local variable 'returnVar' Open
return returnVar;- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
UnnecessaryLocalBeforeReturn
Since: PMD 3.3
Priority: Medium
Categories: Style
Remediation Points: 50000
Avoid the creation of unnecessary local variables
Example:
public class Foo {
public int foo() {
int x = doSomething();
return x; // instead, just 'return doSomething();'
}
}