From 796f1a1fc1eebd19190b4dc995e4461a867a7bb5 Mon Sep 17 00:00:00 2001 From: oowekyala Date: Thu, 27 Jul 2017 17:03:54 +0200 Subject: [PATCH] Split ClassStats' responsibilities --- .../pmd/lang/java/metrics/ClassStats.java | 194 +++--------------- .../pmd/lang/java/metrics/Memoizer.java | 29 +++ .../lang/java/metrics/MetricsComputer.java | 171 +++++++++++++++ .../pmd/lang/java/metrics/OperationStats.java | 40 +--- .../pmd/lang/java/metrics/PackageStats.java | 29 +-- 5 files changed, 249 insertions(+), 214 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/Memoizer.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/MetricsComputer.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/ClassStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/ClassStats.java index f5c83c722e..11e87b8d0d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/ClassStats.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/ClassStats.java @@ -4,44 +4,35 @@ package net.sourceforge.pmd.lang.java.metrics; -import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; -import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.QualifiedName; import net.sourceforge.pmd.lang.java.metrics.signature.FieldSigMask; import net.sourceforge.pmd.lang.java.metrics.signature.FieldSignature; import net.sourceforge.pmd.lang.java.metrics.signature.OperationSigMask; import net.sourceforge.pmd.lang.java.metrics.signature.OperationSignature; -import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey; -import net.sourceforge.pmd.lang.metrics.api.MetricKey; -import net.sourceforge.pmd.lang.metrics.api.MetricVersion; -import net.sourceforge.pmd.lang.metrics.api.ResultOption; /** - * Statistics about a class, enum, interface, or annotation. Gathers information about the contained members and their - * signatures. This class does not provide methods to operate directly on its nested classes, but only on itself. To - * operate on a nested class, retrieve the correct ClassStats with - * {@link PackageStats#getClassStats(QualifiedName, boolean)} then use the methods of ClassStats. + * Statistics about a class, enum, interface, or annotation. Stores information about the contained members and their + * signatures, and memoizes the results of the class metrics computed on the corresponding node. * - *

Note that at this level, entities of the data structure do not manipulate QualifiedNames anymore, only Strings. + *

This class does not provide methods to operate directly on its nested classes, but only on itself. To operate on a + * nested class, retrieve the correct ClassStats with {@link PackageStats#getClassStats(QualifiedName, boolean)} then + * use the methods of ClassStats. Note that at this level, entities of the data structure do not manipulate + * QualifiedNames anymore, only Strings. * * @author Clément Fournier */ -/* default */ class ClassStats { +/* default */ class ClassStats extends Memoizer { private Map> operations = new HashMap<>(); private Map> fields = new HashMap<>(); private Map nestedClasses = new HashMap<>(); - private Map memo = new HashMap<>(); - // References to the hierarchy // TODO:cf useful? // private String superclass; @@ -49,8 +40,8 @@ import net.sourceforge.pmd.lang.metrics.api.ResultOption; /** - * Finds a ClassStats in the direct children of this class. This can only be a directly nested class, for example - * in the following snippet, A can get B and B can get C but A cannot get C without asking B. + * Finds a ClassStats in the direct children of this class. This can only be a directly nested class, for example in + * the following snippet, A can get B and B can get C but A cannot get C without asking B. * 

      * {@code
      * class MyClass { // ClassStats A
@@ -75,6 +66,28 @@ import net.sourceforge.pmd.lang.metrics.api.ResultOption;
     }
 
 
+    OperationStats getOperationStats(String operationName) {
+        for (Map map : operations.values()) {
+            OperationStats stats = map.get(operationName);
+            if (stats != null) {
+                return stats;
+            }
+        }
+        return null;
+    }
+
+
+    OperationStats getOperationStats(String operationName, ASTMethodOrConstructorDeclaration node) {
+        Map sigMap = operations.get(OperationSignature.buildFor(node));
+
+        if (sigMap == null) {
+            return null;
+        }
+
+        return sigMap.get(operationName);
+    }
+
+
     /**
      * Adds an operation to the class.
      *
@@ -146,149 +159,4 @@ import net.sourceforge.pmd.lang.metrics.api.ResultOption;
         return false;
     }
 
-
-    /**
-     * Computes the value of a metric for a class.
-     *
-     * @param key     The class metric for which to find a memoized result
-     * @param node    The AST node of the class
-     * @param force   Force the recomputation; if unset, we'll first check for a memoized result
-     * @param version Version of the metric
-     *
-     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
-     */
-    /* default */ double compute(MetricKey key, ASTAnyTypeDeclaration node, boolean force,
-                                 MetricVersion version) {
-
-        ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version);
-        // if memo.get(key) == null then the metric has never been computed. NaN is a valid value.
-        Double prev = memo.get(paramKey);
-        if (!force && prev != null) {
-            return prev;
-        }
-
-        double val = key.getCalculator().computeFor(node, version);
-        memo.put(paramKey, val);
-
-        return val;
-    }
-
-
-    /**
-     * Computes the value of a metric for an operation.
-     *
-     * @param key     The operation metric for which to find a memoized result
-     * @param node    The AST node of the operation
-     * @param name    The name of the operation
-     * @param force   Force the recomputation; if unset, we'll first check for a memoized result
-     * @param version Version of the metric
-     *
-     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
-     */
-    /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node,
-                                 String name, boolean force, MetricVersion version) {
-
-        // TODO:cf the operation signature might be built many times, consider storing it in the node
-        Map sigMap = operations.get(OperationSignature.buildFor(node));
-
-        if (sigMap == null) {
-            return Double.NaN;
-        }
-
-        OperationStats stats = sigMap.get(name);
-        return stats == null ? Double.NaN : stats.compute(key, node, force, version);
-    }
-
-
-    /**
-     * Computes an aggregate result using a ResultOption.
-     *
-     * @param key     The class metric to compute
-     * @param node    The AST node of the class
-     * @param force   Force the recomputation; if unset, we'll first check for a memoized result
-     * @param version The version of the metric
-     * @param option  The type of result to compute
-     *
-     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
-     */
-    /* default */ double computeWithResultOption(MetricKey key, ASTAnyTypeDeclaration node,
-                                                 boolean force, MetricVersion version, ResultOption option) {
-
-        List ops = findOperations(node, false);
-
-        List values = new ArrayList<>();
-        for (ASTMethodOrConstructorDeclaration op : ops) {
-            if (key.supports(op)) {
-                double val = this.compute(key, op, op.getQualifiedName().getOperation(), force, version);
-                if (val != Double.NaN) {
-                    values.add(val);
-                }
-            }
-        }
-
-        // FUTURE use streams to do that when we upgrade the compiler to 1.8
-        switch (option) {
-        case SUM:
-            return sum(values);
-        case HIGHEST:
-            return highest(values);
-        case AVERAGE:
-            return average(values);
-        default:
-            return Double.NaN;
-        }
-    }
-
-
-    /**
-     * Finds the declaration nodes of all methods or constructors that are declared inside a class.
-     *
-     * @param node          The class in which to look for.
-     * @param includeNested Include operations found in nested classes?
-     *
-     * @return The list of all operations declared inside the specified class.
-     *
-     * TODO:cf this one is computed every time
-     */
-    private static List findOperations(ASTAnyTypeDeclaration node,
-                                                                          boolean includeNested) {
-
-        if (includeNested) {
-            return node.findDescendantsOfType(ASTMethodOrConstructorDeclaration.class);
-        }
-
-        List operations = new ArrayList<>();
-
-        for (ASTAnyTypeBodyDeclaration decl : node.getDeclarations()) {
-            if (decl.jjtGetNumChildren() > 0 && decl.jjtGetChild(0) instanceof ASTMethodOrConstructorDeclaration) {
-                operations.add((ASTMethodOrConstructorDeclaration) decl.jjtGetChild(0));
-            }
-        }
-        return operations;
-    }
-
-
-    private static double sum(List values) {
-        double sum = 0;
-        for (double val : values) {
-            sum += val;
-        }
-        return sum;
-    }
-
-
-    private static double highest(List values) {
-        double highest = Double.NEGATIVE_INFINITY;
-        for (double val : values) {
-            if (val > highest) {
-                highest = val;
-            }
-        }
-        return highest == Double.NEGATIVE_INFINITY ? 0 : highest;
-    }
-
-
-    private static double average(List values) {
-        return sum(values) / values.size();
-    }
 }
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/Memoizer.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/Memoizer.java
new file mode 100644
index 0000000000..548974aa3f
--- /dev/null
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/Memoizer.java
@@ -0,0 +1,29 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.metrics;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey;
+
+/**
+ * @author Clément Fournier
+ */
+public abstract class Memoizer {
+
+
+    private final Map memo = new HashMap<>();
+
+
+    Double getMemo(ParameterizedMetricKey key) {
+        return memo.get(key);
+    }
+
+
+    void memoize(ParameterizedMetricKey key, double value) {
+        memo.put(key, value);
+    }
+}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/MetricsComputer.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/MetricsComputer.java
new file mode 100644
index 0000000000..cd736c131f
--- /dev/null
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/MetricsComputer.java
@@ -0,0 +1,171 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.metrics;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
+import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey;
+import net.sourceforge.pmd.lang.metrics.api.MetricKey;
+import net.sourceforge.pmd.lang.metrics.api.MetricVersion;
+import net.sourceforge.pmd.lang.metrics.api.ResultOption;
+
+/**
+ * Computes a metric. This relieves ClassStats and OperationStats from that responsibility.
+ *
+ * @author Clément Fournier
+ */
+public class MetricsComputer {
+
+    static final MetricsComputer INSTANCE = new MetricsComputer();
+
+
+    /**
+     * Computes the value of a metric for a class and stores the result in the ClassStats object.
+     *
+     * @param key      The class metric to compute
+     * @param node     The AST node of the class
+     * @param force    Force the recomputation; if unset, we'll first check for a memoized result
+     * @param version  The version of the metric to compute
+     * @param memoizer The object memoizing the results
+     *
+     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
+     */
+    /* default */ double compute(MetricKey key, ASTAnyTypeDeclaration node, boolean force,
+                                 MetricVersion version, ClassStats memoizer) {
+
+        ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version);
+        // if memo.get(key) == null then the metric has never been computed. NaN is a valid value.
+        Double prev = memoizer.getMemo(paramKey);
+        if (!force && prev != null) {
+            return prev;
+        }
+
+        double val = key.getCalculator().computeFor(node, version);
+        memoizer.memoize(paramKey, val);
+
+        return val;
+    }
+
+
+    /**
+     * Computes the value of a metric for an operation and stores the result in the OperationStats object.
+     *
+     * @param key      The operation metric to compute
+     * @param node     The AST node of the operation
+     * @param force    Force the recomputation; if unset, we'll first check for a memoized result
+     * @param version  The version of the metric to compute
+     * @param memoizer The object memoizing the results
+     *
+     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
+     */
+    /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node,
+                                 boolean force, MetricVersion version, OperationStats memoizer) {
+
+        ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version);
+        Double prev = memoizer.getMemo(paramKey);
+        if (!force && prev != null) {
+            return prev;
+        }
+
+        double val = key.getCalculator().computeFor(node, version);
+        memoizer.memoize(paramKey, val);
+        return val;
+    }
+
+
+    /**
+     * Computes an aggregate result using a ResultOption.
+     *
+     * @param key     The class metric to compute
+     * @param node    The AST node of the class
+     * @param force   Force the recomputation; if unset, we'll first check for a memoized result
+     * @param version The version of the metric
+     * @param option  The type of result to compute
+     * @param stats   The ClassStats storing info about the class
+     *
+     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
+     */
+    /* default */ double computeWithResultOption(MetricKey key, ASTAnyTypeDeclaration node,
+                                                 boolean force, MetricVersion version, ResultOption option, ClassStats stats) {
+
+        List ops = findOperations(node);
+
+        List values = new ArrayList<>();
+        for (ASTMethodOrConstructorDeclaration op : ops) {
+            if (key.supports(op)) {
+                OperationStats opStats = stats.getOperationStats(op.getQualifiedName().getOperation(), op);
+                double val = this.compute(key, op, force, version, opStats);
+                if (val != Double.NaN) {
+                    values.add(val);
+                }
+            }
+        }
+
+        // FUTURE use streams to do that when we upgrade the compiler to 1.8
+        switch (option) {
+        case SUM:
+            return sum(values);
+        case HIGHEST:
+            return highest(values);
+        case AVERAGE:
+            return average(values);
+        default:
+            return Double.NaN;
+        }
+    }
+
+
+    /**
+     * Finds the declaration nodes of all methods or constructors that are declared inside a class.
+     *
+     * @param node The class in which to look for.
+     *
+     * @return The list of all operations declared inside the specified class.
+     *
+     * TODO:cf this one is computed every time
+     */
+    private static List findOperations(ASTAnyTypeDeclaration node) {
+
+        List operations = new ArrayList<>();
+
+        for (ASTAnyTypeBodyDeclaration decl : node.getDeclarations()) {
+            if (decl.jjtGetNumChildren() > 0 && decl.jjtGetChild(0) instanceof ASTMethodOrConstructorDeclaration) {
+                operations.add((ASTMethodOrConstructorDeclaration) decl.jjtGetChild(0));
+            }
+        }
+        return operations;
+    }
+
+
+    private static double sum(List values) {
+        double sum = 0;
+        for (double val : values) {
+            sum += val;
+        }
+        return sum;
+    }
+
+
+    private static double highest(List values) {
+        double highest = Double.NEGATIVE_INFINITY;
+        for (double val : values) {
+            if (val > highest) {
+                highest = val;
+            }
+        }
+        return highest == Double.NEGATIVE_INFINITY ? 0 : highest;
+    }
+
+
+    private static double average(List values) {
+        return sum(values) / values.size();
+    }
+
+
+}
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/OperationStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/OperationStats.java
index f860c5540e..96410dc748 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/OperationStats.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/OperationStats.java
@@ -4,60 +4,26 @@
 
 package net.sourceforge.pmd.lang.java.metrics;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
-import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey;
-import net.sourceforge.pmd.lang.metrics.api.MetricKey;
-import net.sourceforge.pmd.lang.metrics.api.MetricVersion;
-
-
 /**
  * Statistics for an operation. Keeps a map of all memoized metrics results.
  *
  * @author Clément Fournier
  */
-/* default */ class OperationStats {
+class OperationStats extends Memoizer {
 
     private final String name;
-    private final Map memo = new HashMap<>();
 
 
-    /* default */ OperationStats(String name) {
+    OperationStats(String name) {
         this.name = name;
     }
 
 
-    /* default */ String getName() {
+    String getName() {
         return name;
     }
 
 
-    /**
-     * Computes the value of a metric for an operation.
-     *
-     * @param key   The operation metric for which to find a memoized result
-     * @param node  The AST node of the operation
-     * @param force Force the recomputation; if unset, we'll first check for a memoized result
-     *
-     * @return The result of the computation, or {@code Double.NaN} if it couldn't be performed
-     */
-    /* default */ double compute(MetricKey key, ASTMethodOrConstructorDeclaration node,
-                                 boolean force, MetricVersion version) {
-
-        ParameterizedMetricKey paramKey = ParameterizedMetricKey.getInstance(key, version);
-        Double prev = memo.get(paramKey);
-        if (!force && prev != null) {
-            return prev;
-        }
-
-        double val = key.getCalculator().computeFor(node, version);
-        memo.put(paramKey, val);
-        return val;
-    }
-
-
     @Override
     public boolean equals(Object o) {
         if (this == o) {
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/PackageStats.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/PackageStats.java
index 632c01602e..bbfa5907b4 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/PackageStats.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/PackageStats.java
@@ -18,8 +18,8 @@ import net.sourceforge.pmd.lang.metrics.api.ResultOption;
 
 
 /**
- * Statistics about a package. This recursive data structure mirrors the package structure of the analysed
- * project and stores information about the classes and subpackages it contains.
+ * Statistics about a package. This recursive data structure mirrors the package structure of the analysed project and
+ * stores information about the classes and subpackages it contains.
  *
  * @author Clément Fournier
  * @see ClassStats
@@ -48,8 +48,7 @@ public final class PackageStats {
 
 
     /**
-     * Returns true if the signature of the operation designated by the qualified name is covered by
-     * the mask.
+     * Returns true if the signature of the operation designated by the qualified name is covered by the mask.
      *
      * @param qname   The operation to test
      * @param sigMask The signature mask to use
@@ -64,8 +63,8 @@ public final class PackageStats {
 
 
     /**
-     * Gets the ClassStats corresponding to the named resource. The class can be nested. If the
-     * createIfNotFound parameter is set, the method also creates the hierarchy if it doesn't exist.
+     * Gets the ClassStats corresponding to the named resource. The class can be nested. If the createIfNotFound
+     * parameter is set, the method also creates the hierarchy if it doesn't exist.
      *
      * @param qname            The qualified name of the class
      * @param createIfNotFound Create hierarchy if missing
@@ -102,8 +101,8 @@ public final class PackageStats {
 
 
     /**
-     * Returns the deepest PackageStats that contains the named resource. If the second parameter is
-     * set, creates the missing PackageStats along the way.
+     * Returns the deepest PackageStats that contains the named resource. If the second parameter is set, creates the
+     * missing PackageStats along the way.
      *
      * @param qname            The qualified name of the resource
      * @param createIfNotFound If set to true, the hierarch is created if non existent
@@ -131,8 +130,8 @@ public final class PackageStats {
 
 
     /**
-     * Returns true if the signature of the field designated by its name and the qualified name of its class is
-     * covered by the mask.
+     * Returns true if the signature of the field designated by its name and the qualified name of its class is covered
+     * by the mask.
      *
      * @param qname     The class of the field
      * @param fieldName The name of the field
@@ -162,7 +161,7 @@ public final class PackageStats {
         ClassStats container = getClassStats(node.getQualifiedName(), false);
 
         return container == null ? Double.NaN
-                                 : container.compute(key, node, force, version);
+                                 : MetricsComputer.INSTANCE.compute(key, node, force, version, container);
     }
 
 
@@ -180,9 +179,10 @@ public final class PackageStats {
                                  boolean force, MetricVersion version) {
         QualifiedName qname = node.getQualifiedName();
         ClassStats container = getClassStats(qname, false);
+        OperationStats memoizer = container == null ? null : container.getOperationStats(qname.getOperation(), node);
 
-        return container == null ? Double.NaN
-                                 : container.compute(key, node, qname.getOperation(), force, version);
+        return memoizer == null ? Double.NaN
+                                : MetricsComputer.INSTANCE.compute(key, node, force, version, memoizer);
     }
 
 
@@ -202,6 +202,7 @@ public final class PackageStats {
         ClassStats container = getClassStats(node.getQualifiedName(), false);
 
         return container == null ? Double.NaN
-                                 : container.computeWithResultOption(key, node, force, version, option);
+                                 : MetricsComputer.INSTANCE.computeWithResultOption(key, node, force, version,
+                                                                                    option, container);
     }
 }