wissel.net

Usability - Productivity - Business - The web - Singapore & Twins

Avoid the "Clean Code Shock" with PMD


Your new year resolution includes "Write clean Apex code". So you run PMD with a full ruleset and get shocked by the number of violations. You drop the resolution in a blink.

Don't boil the Ocean

Even a journey of a thousand miles starts with a single step, so let's break down the task into manageable chunks to divide and rule.

There are 2 dimensions you can use: Type of code and priority levels. Using them you can turn your Clean Code journey into manageable stages.

Code Types

  • Legacy code: all code that doesn't fall in any of the two other categories
  • Changed code: code that needs change due to business requirements
  • New code: new code written for new or changed functionality (applies to copy & paste too)

Priority Levels

  • 1 = security and performance, will fail build
  • 2 = bad code, will fail build
  • 3 & 4 = hard to maintain code, will generate warning
  • 5 = ugly code, will generate hint

PMD rules for code types have different priorities. A different number of tests will fail a build:

  • 11 for legacy code (all around performance and security)
  • 33 for changed code
  • 44 for new code

A pragmatic rule set

Rule Set Rule Name Legacy code Updated Code New Code
Best Practices ApexUnitTestShouldNotUseSeeAllDataTrue 3 2 2
ApexUnitTestClassShouldHaveAsserts 3 2 2
AvoidLogicInTrigger 2 2 2
AvoidGlobalModifier 5 4 3
Code Style ClassNamingConventions 5 3 1
ForLoopsMustUseBraces 4 2 1
IfStmtsMustUseBraces 4 2 1
IfElseStmtsMustUseBraces 5 2 1
MethodNamingConventions 5 3 1
OneDeclarationPerLine 5 2 1
VariableNamingConventions 5 3 1
WhileLoopsMustUseBraces 4 2 1
Design AvoidDeeplyNestedIfStmts 3 2 2
CyclomaticComplexity 3 3 2
ExcessiveClassLength 3 3 2
ExcessiveParameterList 3 3 2
ExcessivePublicCount 3 3 2
NcssConstructorCount 3 2 2
NcssMethodCount 3 3 2
NcssTypeCount 3 3 2
StdCyclomaticComplexity 3 3 2
TooManyFields 3 3 3
Error Prone AvoidDirectAccessTriggerMap 2 2 1
AvoidHardcodingId 2 2 1
EmptyCatchBlock 2 2 2
EmptyIfStmt 3 2 2
EmptyStatementBlock 3 3 2
EmptyTryOrFinallyBlock 3 3 2
EmptyWhileStmt 3 3 2
MethodWithSameNameAsEnclosingClass 3 3 2
Performance AvoidDmlStatementsInLoops 1 1 1
AvoidSoslInLoops 1 1 1
AvoidSoqlInLoops 1 1 1
Security ApexCRUDViolation 3 2 1
ApexInsecureEndpoint 2 2 2
ApexOpenRedirect 3 2 2
ApexSharingViolations 3 3 2
ApexSOQLInjection 1 1 1
ApexXSSFromURLParam 1 1 1
VfCsrf 1 1 1
ApexXSSFromEscapeFalse 3 2 1
ApexBadCrypto 1 1 1
ApexCSRF 3 2 2
ApexDangerousMethods 3 2 2
ApexSuggestUsingNamedCred 1 1 1
VfUnescapeEl 1 1 1

As usual: YMMV - You need to tune the rule to match your code quality of your existing base. The most controversial would be the "updated code" rule. If a business requirement mandates to add 3-4 lines, but the PMD rule would fail on complexity, it becomes hard to justify to upfront work, so you might, initially, have more relaxed rules in place.


Posted by on 02 January 2019 | Comments (0) | categories: Apex PMD Salesforce

Comments

  1. No comments yet, be the first to comment