From a57741b72652f11c1f407eb99ec351a0400e3e4f Mon Sep 17 00:00:00 2001
From: Joel Therrien <joel@joeltherrien.ca>
Date: Thu, 10 Jan 2019 11:23:55 -0800
Subject: [PATCH] Add PMD rules to pom.xml to enforce higher code quality

---
 pmd-rules.xml                                 | 93 +++++++++++++++++++
 pom.xml                                       | 23 ++++-
 .../joeltherrien/randomforest/DataLoader.java |  2 +-
 .../covariates/BooleanCovariate.java          |  2 +-
 .../randomforest/covariates/Covariate.java    |  4 +-
 .../covariates/FactorCovariate.java           |  1 -
 .../CompetingRiskGraySetsImpl.java            |  2 +-
 .../CompetingRiskGroupDifferentiator.java     |  6 --
 .../randomforest/tree/ForestTrainer.java      |  2 +-
 .../randomforest/tree/TreeTrainer.java        |  7 +-
 .../utils/UniqueSubsetValueIterator.java      |  4 -
 11 files changed, 124 insertions(+), 22 deletions(-)
 create mode 100644 pmd-rules.xml

diff --git a/pmd-rules.xml b/pmd-rules.xml
new file mode 100644
index 0000000..abad748
--- /dev/null
+++ b/pmd-rules.xml
@@ -0,0 +1,93 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<ruleset name="Default Maven PMD Plugin Ruleset"
+    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
+    <description>
+        The default ruleset used by the Maven PMD Plugin, when no other ruleset is specified.
+        It contains the rules of the old (pre PMD 6.0.0) rulesets java-basic, java-empty, java-imports,
+        java-unnecessary, java-unusedcode.
+
+        This ruleset might be used as a starting point for an own customized ruleset [0].
+
+        [0] https://pmd.github.io/latest/pmd_userdocs_understanding_rulesets.html
+    </description>
+
+    <rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP" />
+    <rule ref="category/java/bestpractices.xml/CheckResultSet" />
+    <rule ref="category/java/bestpractices.xml/UnusedImports" />
+    <rule ref="category/java/bestpractices.xml/UnusedFormalParameter" />
+    <rule ref="category/java/bestpractices.xml/UnusedLocalVariable" />
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateField" />
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod" />
+
+    <rule ref="category/java/codestyle.xml/DontImportJavaLang" />
+    <rule ref="category/java/codestyle.xml/DuplicateImports" />
+    <rule ref="category/java/codestyle.xml/ExtendsObject" />
+    <rule ref="category/java/codestyle.xml/ForLoopShouldBeWhileLoop" />
+    <rule ref="category/java/codestyle.xml/TooManyStaticImports" />
+    <rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName" />
+    <rule ref="category/java/codestyle.xml/UnnecessaryModifier" />
+    <rule ref="category/java/codestyle.xml/UnnecessaryReturn" />
+    <!--<rule ref="category/java/codestyle.xml/UselessParentheses" /> Sometimes parentheses are used for human eyes -->
+    <rule ref="category/java/codestyle.xml/UselessQualifiedThis" />
+
+    <rule ref="category/java/design.xml/CollapsibleIfStatements" />
+    <rule ref="category/java/design.xml/SimplifiedTernary" />
+    <rule ref="category/java/design.xml/UselessOverridingMethod" />
+
+    <rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop" />
+    <rule ref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor" />
+    <rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators" />
+    <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues" />
+    <rule ref="category/java/errorprone.xml/BrokenNullCheck" />
+    <rule ref="category/java/errorprone.xml/CheckSkipResult" />
+    <rule ref="category/java/errorprone.xml/ClassCastExceptionWithToArray" />
+    <rule ref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices" />
+    <rule ref="category/java/errorprone.xml/EmptyCatchBlock" />
+    <rule ref="category/java/errorprone.xml/EmptyFinallyBlock" />
+    <rule ref="category/java/errorprone.xml/EmptyIfStmt" />
+    <rule ref="category/java/errorprone.xml/EmptyInitializer" />
+    <rule ref="category/java/errorprone.xml/EmptyStatementBlock" />
+    <rule ref="category/java/errorprone.xml/EmptyStatementNotInLoop" />
+    <rule ref="category/java/errorprone.xml/EmptySwitchStatements" />
+    <rule ref="category/java/errorprone.xml/EmptySynchronizedBlock" />
+    <rule ref="category/java/errorprone.xml/EmptyTryBlock" />
+    <rule ref="category/java/errorprone.xml/EmptyWhileStmt" />
+    <rule ref="category/java/errorprone.xml/ImportFromSamePackage" />
+    <rule ref="category/java/errorprone.xml/JumbledIncrementer" />
+    <rule ref="category/java/errorprone.xml/MisplacedNullCheck" />
+    <rule ref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode" />
+    <rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock" />
+    <rule ref="category/java/errorprone.xml/UnconditionalIfStatement" />
+    <rule ref="category/java/errorprone.xml/UnnecessaryConversionTemporary" />
+    <rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals" />
+    <rule ref="category/java/errorprone.xml/UselessOperationOnImmutable" />
+
+    <rule ref="category/java/multithreading.xml/AvoidThreadGroup" />
+    <rule ref="category/java/multithreading.xml/DontCallThreadRun" />
+    <rule ref="category/java/multithreading.xml/DoubleCheckedLocking" />
+
+    <rule ref="category/java/performance.xml/BigIntegerInstantiation" />
+    <rule ref="category/java/performance.xml/BooleanInstantiation" />
+
+</ruleset>
\ No newline at end of file
diff --git a/pom.xml b/pom.xml
index 5d6ba5b..6d22d2b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -13,6 +13,9 @@
         <maven.compiler.target>1.8</maven.compiler.target>
         <maven.compiler.source>1.8</maven.compiler.source>
         <jackson.version>2.9.6</jackson.version>
+
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     </properties>
 
 
@@ -89,9 +92,27 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-pmd-plugin</artifactId>
+                <version>3.11.0</version>
+                <executions>
+                    <execution>
+                        <phase>package</phase> <!-- bind to the packaging phase -->
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <rulesets>
+                        <!-- Custom local file system rule set -->
+                        <ruleset>${project.basedir}/pmd-rules.xml</ruleset>
+                    </rulesets>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 
 
-
 </project>
\ No newline at end of file
diff --git a/src/main/java/ca/joeltherrien/randomforest/DataLoader.java b/src/main/java/ca/joeltherrien/randomforest/DataLoader.java
index 25e8465..5118fdc 100644
--- a/src/main/java/ca/joeltherrien/randomforest/DataLoader.java
+++ b/src/main/java/ca/joeltherrien/randomforest/DataLoader.java
@@ -58,7 +58,7 @@ public class DataLoader {
             throw new IllegalArgumentException("Tree directory must be a directory!");
         }
 
-        final File[] treeFiles = folder.listFiles(((file, s) -> s.endsWith(".tree")));
+        final File[] treeFiles = folder.listFiles((file, s) -> s.endsWith(".tree"));
         final List<File> treeFileList = Arrays.asList(treeFiles);
 
         Collections.sort(treeFileList, Comparator.comparing(File::getName));
diff --git a/src/main/java/ca/joeltherrien/randomforest/covariates/BooleanCovariate.java b/src/main/java/ca/joeltherrien/randomforest/covariates/BooleanCovariate.java
index 85221a0..480485e 100644
--- a/src/main/java/ca/joeltherrien/randomforest/covariates/BooleanCovariate.java
+++ b/src/main/java/ca/joeltherrien/randomforest/covariates/BooleanCovariate.java
@@ -21,7 +21,7 @@ public final class BooleanCovariate implements Covariate<Boolean> {
 
     @Override
     public <Y> Iterator<Split<Y, Boolean>> generateSplitRuleUpdater(List<Row<Y>> data, int number, Random random) {
-        return new SingletonIterator<>(BooleanCovariate.this.splitRule.applyRule(data));
+        return new SingletonIterator<>(this.splitRule.applyRule(data));
     }
 
     @Override
diff --git a/src/main/java/ca/joeltherrien/randomforest/covariates/Covariate.java b/src/main/java/ca/joeltherrien/randomforest/covariates/Covariate.java
index 6e9b94e..1f04a0f 100644
--- a/src/main/java/ca/joeltherrien/randomforest/covariates/Covariate.java
+++ b/src/main/java/ca/joeltherrien/randomforest/covariates/Covariate.java
@@ -66,7 +66,7 @@ public interface Covariate<V> extends Serializable {
 
 
             for(final Row<Y> row : rows) {
-                final Value<V> value = (Value<V>) row.getCovariateValue(getParent());
+                final Value<V> value = row.getCovariateValue(getParent());
 
                 if(value.isNA()){
                     missingValueRows.add(row);
@@ -88,7 +88,7 @@ public interface Covariate<V> extends Serializable {
         }
 
         default boolean isLeftHand(CovariateRow row, final double probabilityNaLeftHand){
-            final Value<V> value = (Value<V>) row.getCovariateValue(getParent());
+            final Value<V> value = row.getCovariateValue(getParent());
 
             if(value.isNA()){
                 return ThreadLocalRandom.current().nextDouble() <= probabilityNaLeftHand;
diff --git a/src/main/java/ca/joeltherrien/randomforest/covariates/FactorCovariate.java b/src/main/java/ca/joeltherrien/randomforest/covariates/FactorCovariate.java
index fd00c7e..89f7a02 100644
--- a/src/main/java/ca/joeltherrien/randomforest/covariates/FactorCovariate.java
+++ b/src/main/java/ca/joeltherrien/randomforest/covariates/FactorCovariate.java
@@ -6,7 +6,6 @@ import lombok.EqualsAndHashCode;
 import lombok.Getter;
 
 import java.util.*;
-import java.util.concurrent.ThreadLocalRandom;
 
 public final class FactorCovariate implements Covariate<String>{
 
diff --git a/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/CompetingRiskGraySetsImpl.java b/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/CompetingRiskGraySetsImpl.java
index 6650dcb..a3b6f31 100644
--- a/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/CompetingRiskGraySetsImpl.java
+++ b/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/CompetingRiskGraySetsImpl.java
@@ -21,7 +21,7 @@ public class CompetingRiskGraySetsImpl implements CompetingRiskSets{
 
     @Override
     public MathFunction getRiskSet(int event){
-        return(riskSet[event-1]);
+        return riskSet[event-1];
     }
 
     @Override
diff --git a/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/differentiator/CompetingRiskGroupDifferentiator.java b/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/differentiator/CompetingRiskGroupDifferentiator.java
index b09026a..d925094 100644
--- a/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/differentiator/CompetingRiskGroupDifferentiator.java
+++ b/src/main/java/ca/joeltherrien/randomforest/responses/competingrisk/differentiator/CompetingRiskGroupDifferentiator.java
@@ -2,12 +2,10 @@ package ca.joeltherrien.randomforest.responses.competingrisk.differentiator;
 
 import ca.joeltherrien.randomforest.responses.competingrisk.CompetingRiskResponse;
 import ca.joeltherrien.randomforest.responses.competingrisk.CompetingRiskSets;
-import ca.joeltherrien.randomforest.tree.GroupDifferentiator;
 import ca.joeltherrien.randomforest.tree.SimpleGroupDifferentiator;
 import lombok.AllArgsConstructor;
 import lombok.Data;
 
-import java.util.List;
 import java.util.stream.Stream;
 
 /**
@@ -59,10 +57,6 @@ public abstract class CompetingRiskGroupDifferentiator<Y extends CompetingRiskRe
                 summation += deltaSummation;
                 variance += deltaVariance;
             }
-            else{
-                // Do nothing; else statement left for breakpoints.
-            }
-
         }
 
         return new LogRankValue(summation, variance);
diff --git a/src/main/java/ca/joeltherrien/randomforest/tree/ForestTrainer.java b/src/main/java/ca/joeltherrien/randomforest/tree/ForestTrainer.java
index 900fdfd..b6330cc 100644
--- a/src/main/java/ca/joeltherrien/randomforest/tree/ForestTrainer.java
+++ b/src/main/java/ca/joeltherrien/randomforest/tree/ForestTrainer.java
@@ -132,7 +132,7 @@ public class ForestTrainer<Y, TO, FO> {
         }
 
 
-        final File[] treeFiles = folder.listFiles(((file, s) -> s.endsWith(".tree")));
+        final File[] treeFiles = folder.listFiles((file, s) -> s.endsWith(".tree"));
 
         final ExecutorService executorService = Executors.newFixedThreadPool(threads);
         final AtomicInteger treeCount = new AtomicInteger(treeFiles.length); // tracks how many trees are finished
diff --git a/src/main/java/ca/joeltherrien/randomforest/tree/TreeTrainer.java b/src/main/java/ca/joeltherrien/randomforest/tree/TreeTrainer.java
index 0318cef..27589f8 100644
--- a/src/main/java/ca/joeltherrien/randomforest/tree/TreeTrainer.java
+++ b/src/main/java/ca/joeltherrien/randomforest/tree/TreeTrainer.java
@@ -134,10 +134,9 @@ public class TreeTrainer<Y, O> {
 
             final SplitAndScore<Y, ?> candidateSplitAndScore = noGenericDifferentiator.differentiate(iterator);
 
-            if(candidateSplitAndScore != null) {
-                if (bestSplitAndScore == null || candidateSplitAndScore.getScore() > bestSplitAndScore.getScore()) {
-                    bestSplitAndScore = candidateSplitAndScore;
-                }
+            if(candidateSplitAndScore != null && (bestSplitAndScore == null ||
+                    candidateSplitAndScore.getScore() > bestSplitAndScore.getScore())) {
+                bestSplitAndScore = candidateSplitAndScore;
             }
 
         }
diff --git a/src/main/java/ca/joeltherrien/randomforest/utils/UniqueSubsetValueIterator.java b/src/main/java/ca/joeltherrien/randomforest/utils/UniqueSubsetValueIterator.java
index ebce035..0b7b481 100644
--- a/src/main/java/ca/joeltherrien/randomforest/utils/UniqueSubsetValueIterator.java
+++ b/src/main/java/ca/joeltherrien/randomforest/utils/UniqueSubsetValueIterator.java
@@ -1,10 +1,6 @@
 package ca.joeltherrien.randomforest.utils;
 
 
-import lombok.Getter;
-
-import java.util.Iterator;
-
 /**
  * Iterator that wraps around a UniqueValueIterator. It continues to iterate until it gets to one of the prespecified indexes,
  * and then proceeds just past that to the end of the existing values it's at.