Apex error prone best practices

This article is based on the PMD documentation article. See the original article on the PMD site: PMD: error prone best practices.

Best practices to detect constructs that are either broken, extremely confusing or prone to runtime errors.

AvoidDirectAccessTriggerMap

Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Triggers should be bulkified and iterate through the map to handle the actions for each item separately.

//ArrayLoadExpression[TriggerVariableExpression and LiteralExpression]

Example

trigger AccountTrigger on Account (before insert, before update) {
   Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
   
   foreach ( Account a : Trigger.new ){   
        //Good: Iterate through the trigger.new array instead.
   }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

AvoidHardcodingId

When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, the logic can dynamically identify the proper data to operate against and not fail.

This rule is defined by the following Java class:

net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidHardcodingIdRule

Example

public without sharing class Foo {
    void foo() {
        //Error - hardcoded the record type id
        if(a.RecordTypeId == '012500000009WAr'){
            //do some logic here.....
        } else if(a.RecordTypeId == '0123000000095Km'){
            //do some logic here for a different record type...
        }
    }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

AvoidTrailingComma

This rule helps improve code portability due to differences in browser treatment of trailing commas in object or array literals.

This rule is defined by the following XPath expression:

//ObjectLiteral[$allowObjectLiteral = false() and @TrailingComma = true()]
|
//ArrayLiteral[$allowArrayLiteral = false() and @TrailingComma = true()]

Example

function(arg) {
    var obj1 = { a : 1 };   // Ok
    var arr1 = [ 1, 2 ];    // Ok

    var obj2 = { a : 1, };  // Syntax error in some browsers!
    var arr2 = [ 1, 2, ];   // Length 2 or 3 depending on the browser!
}

Properties

NameDefault ValueDescriptionMultivalued
allowObjectLiteralfalseAllow a trailing comma within an object literalno
allowArrayLiteralfalseAllow a trailing comma within an array literalno

EmptyCatchBlock

Empty Catch Block finds instances where an exception is caught, but nothing is done.
In most circumstances, this swallows an exception which should either be acted on or reported.

//CatchBlockStatement[./BlockStatement[count(*) = 0]]

Example

public void doSomething() {
  ...
  try {
    insert accounts;
  } catch (DmlException dmle) {
    // not good
  }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

EmptyIfStmt

Empty If Statement finds instances where a condition is checked but nothing is done about it.

//IfBlockStatement
 [BlockStatement[count(*) = 0]]

Example

public class Foo {
  public void bar(Integer x) {
    if (x == 0) {
      // empty!
    }
  }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

EmptyStatementBlock

Empty block statements serve no purpose and should be removed.

//Method/ModifierNode[@Abstract!='true' and ../BlockStatement[count(*) = 0]]
| //Method/BlockStatement//BlockStatement[count(*) = 0 and @Location != parent::*/@Location]

Example

public
 class Foo {

   private int _bar;

   public void setBar(int bar) {
        // empty
   }

}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

EmptyTryOrFinallyBlock

Avoid empty try or finally blocks - what’s the point?

//TryCatchFinallyBlockStatement[./BlockStatement[count(*) = 0]]

Example

public class Foo {
    public void bar() {
        try {
          // empty !
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

public class Foo {
    public void bar() {
        try {
            int x=2;
        } finally {
            // empty!
        }
    }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

EmptyWhileStmt

Empty While Statement finds all instances where a while statement does nothing.
If it is a timing loop, then you should use Thread.sleep() for it; if it is a while loop that does a lot in the exit expression, rewrite it to make it clearer.

//WhileLoopStatement[./BlockStatement[count(*) = 0]]

Example

public void bar(Integer a, Integer b) {
  while (a == b) {
    // empty!
  }
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

EqualComparison

Using == in condition may lead to unexpected results, as the variables are automatically casted to be of the same type. The === operator avoids the casting.

This rule is defined by the following XPath expression:

//InfixExpression[
  (@Image = "==" or @Image = "!=")
  and
  (KeywordLiteral[@Image='true' or @Image = 'false'] or NumberLiteral)
]

Example

// Ok
if (someVar === true) {
  ...
}
// Ok
if (someVar !== 3) {
  ...
}
// Bad
if (someVar == true) {
  ...
}
// Bad
if (someVar != 3) {
  ...
}


InnaccurateNumericLiteral

The numeric literal will have a different value at runtime, which can happen if you provide too much precision in a floating point number. This may result in numeric calculations being in error.

This rule is defined by the following XPath expression:

//NumberLiteral[@NormalizedImage != string(@Number)]

Example

var a = 9; // Ok
var b = 999999999999999; // Ok
var c = 999999999999999999999; // Not good
var w = 1.12e-4; // Ok
var x = 1.12; // Ok
var y = 1.1234567890123; // Ok
var z = 1.12345678901234567; // Not good


MethodWithSameNameAsEnclosingClass

Non-constructor methods should not have the same name as the enclosing class.

This rule is defined by the following Java class:

net.sourceforge.pmd.lang.apex.rule.errorprone.MethodWithSameNameAsEnclosingClassRule

Example

public class MyClass {
    // this is OK because it is a constructor
    public MyClass() {}
    // this is bad because it is a method
    public void MyClass() {}
}

Properties

NameDefault ValueDescription
cc_categories[Style]Code Climate Categories
cc_remediation_points_multiplier1Code Climate Remediation Points multiplier
cc_block_highlightingfalseCode Climate Block Highlighting

What's here

Related content

 PMD: error prone best practices




Last modified on Jul 14, 2020