Refactoring hotspots

lib/metric_fu/metrics/churn/hotspot.rb

@@ -27,10 +27,11 @@ def calculate_score(metric_ranking, item)
     metric_ranking.scored?(item) ? flat_churn_score : 0
   end
 
-  def generate_records(data, table)
-   return if data==nil
-    Array(data[:changes]).each do |change|
-      table << {
+  # @return [Array<Hash>]
+  def generate_records(data)
+   return [] if data==nil
+    Array(data[:changes]).map do |change|
+      {
         "metric" => :churn,
         "times_changed" => change[:times_changed],
         "file_path" => change[:file_path]

lib/metric_fu/metrics/flay/hotspot.rb

@@ -22,9 +22,9 @@ def score_strategy
     :percentile
   end
 
-  def generate_records(data, table)
-    return if data==nil
-    Array(data[:matches]).each do |match|
+  def generate_records(data)
+    return [] if data==nil
+    Array(data[:matches]).flat_map do |match|
       problems  = match[:reason]
       matching_reason = problems.gsub(/^[0-9]+\) /,'').gsub(/\:[0-9]+/,'')
       files     = []
@@ -35,8 +35,8 @@ def generate_records(data, table)
         files     << file_path
       end
       files = files.uniq
-      files.each do |file|
-        table << {
+      files.map do |file|
+        {
           "metric" => self.name,
           "file_path" => file,
           "flay_reason" => problems+" files: #{locations.join(', ')}",

lib/metric_fu/metrics/flog/hotspot.rb

@@ -22,13 +22,13 @@ def score_strategy
     :identity
   end
 
-  def generate_records(data, table)
-    return if data==nil
-    Array(data[:method_containers]).each do |method_container|
-      Array(method_container[:methods]).each do |entry|
+  def generate_records(data)
+    return [] if data==nil
+    Array(data[:method_containers]).flat_map do |method_container|
+      Array(method_container[:methods]).map do |entry|
         file_path = entry[1][:path].sub(%r{^/},'') if entry[1][:path]
         location = MetricFu::Location.for(entry.first)
-        table << {
+        {
           "metric" => name,
           "score" => entry[1][:score],
           "file_path" => file_path,

lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb

@@ -13,6 +13,10 @@ def generate_records
       process_rows!
     end
 
+    def update(generated_records)
+      table.concat generated_records
+    end
+
     def tool_tables
       @tool_tables ||= make_table_hash(@columns)
     end

lib/metric_fu/metrics/hotspots/analysis/table.rb

@@ -25,6 +25,13 @@ def <<(row)
       updated_key_index(record) if @make_index
     end
 
+    def concat(records)
+      records.each do |record|
+        self << record
+      end
+      self
+    end
+
     def each
       @rows.each do |row|
         yield row

lib/metric_fu/metrics/hotspots/hotspot.rb

@@ -69,9 +69,8 @@ def score_strategy
 
     # Transforms the data param, if non-nil, into a hash with keys:
     #   'metric',  etc.
-    #   and appends the hash to the table param
-    #   Has no return value
-    def generate_records(data, table)
+    # @return [Array<Hash>]
+    def generate_records(data)
       not_implemented
     end
 

lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb

@@ -50,7 +50,7 @@ def setup(result_hash)
       # to ultimately generate the hotspots
       @analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns)
       tool_analyzers.each do |analyzer|
-        analyzer.generate_records(result_hash[analyzer.name], @analyzer_tables.table)
+        @analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name])
       end
       @analyzer_tables.generate_records
       @rankings = MetricFu::HotspotRankings.new(@analyzer_tables.tool_tables)

lib/metric_fu/metrics/rcov/hotspot.rb

@@ -22,21 +22,21 @@ def score_strategy
     :identity
   end
 
-  def generate_records(data, table)
-    return if data==nil
-    data.each do |file_name, info|
+  def generate_records(data)
+    return [] if data==nil
+    data.flat_map do |file_name, info|
       next if (file_name == :global_percent_run) || (info[:methods].nil?)
-      info[:methods].each do |method_name, percentage_uncovered|
+      info[:methods].map do |method_name, percentage_uncovered|
         location = MetricFu::Location.for(method_name)
-        table << {
+        {
           "metric" => :rcov,
           'file_path' => file_name,
           'class_name' => location.class_name,
           "method_name" => location.method_name,
           "percentage_uncovered" => percentage_uncovered
          }
       end
-    end
+    end.compact
   end
 
   def present_group(group)

lib/metric_fu/metrics/reek/hotspot.rb

@@ -26,15 +26,15 @@ def score_strategy
     :percentile
   end
 
-  def generate_records(data, table)
-    return if data==nil
-    data[:matches].each do |match|
+  def generate_records(data)
+    return [] if data==nil
+    data[:matches].flat_map do |match|
       file_path = match[:file_path]
-      match[:code_smells].each do |smell|
+      match[:code_smells].map do |smell|
         location = MetricFu::Location.for(smell[:method])
         smell_type = smell[:type]
         message = smell[:message]
-        table << {
+        {
           "metric" => name, # important
           "file_path" => file_path, # important
           # NOTE: ReekHotspot is currently different than other hotspots with regard

lib/metric_fu/metrics/roodi/hotspot.rb

@@ -22,10 +22,10 @@ def score_strategy
     :percentile
   end
 
-  def generate_records(data, table)
-    return if data==nil
-    Array(data[:problems]).each do |problem|
-      table << {
+  def generate_records(data)
+    return [] if data==nil
+    Array(data[:problems]).map do |problem|
+      {
         "metric" => name,
         "problems" => problem[:problem],
         "file_path" => problem[:file]

lib/metric_fu/metrics/saikuro/hotspot.rb

@@ -22,16 +22,16 @@ def score_strategy
     :identity
   end
 
-  def generate_records(data, table)
-    return if data == nil
-    data[:files].each do |file|
+  def generate_records(data)
+    return [] if data == nil
+    data[:files].flat_map do |file|
       file_name = file[:filename]
-      file[:classes].each do |klass|
+      file[:classes].flat_map do |klass|
         location = MetricFu::Location.for(klass[:class_name])
         offending_class = location.class_name
-        klass[:methods].each do |match|
+        klass[:methods].map do |match|
           offending_method = MetricFu::Location.for(match[:name]).method_name
-          table << {
+          {
             "metric" => name,
             "lines" => match[:lines],
             "complexity" => match[:complexity],

lib/metric_fu/metrics/stats/hotspot.rb

@@ -22,16 +22,16 @@ def score_strategy
     :absent
   end
 
-  def generate_records(data, table)
-    return if data == nil
-    data.each do |key, value|
+  def generate_records(data)
+    return [] if data == nil
+    data.map do |key, value|
       next if value.kind_of?(Array)
-      table << {
+      {
         "metric" => name,
         "stat_name" => key,
         "stat_value" => value
       }
-    end
+    end.compact
   end
 
 end

spec/metric_fu/metrics/hotspots/analysis/analyzed_problems_spec.rb

@@ -17,7 +17,7 @@ def analyzed_problems(result_hash)
 
       analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns)
       tool_analyzers.each do |analyzer|
-        analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table)
+        analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name])
       end
       analyzer_tables.generate_records
       rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables)

spec/metric_fu/metrics/hotspots/analysis/analyzer_tables_spec.rb

@@ -17,7 +17,7 @@ def analyzer_table(result_hash)
 
       analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns)
       tool_analyzers.each do |analyzer|
-        analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table)
+        analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name])
       end
       analyzer_tables.generate_records
       rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables)

spec/metric_fu/metrics/hotspots/analysis/rankings_spec.rb

@@ -17,8 +17,8 @@ def rankings(result_hash)
       analyzer_columns = common_columns + granularities + tool_analyzers.map{|analyzer| analyzer.columns}.flatten
 
       analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns)
-      tool_analyzers.each do |analyzer|
-        analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table)
+      tool_analyzers.each do |hotspot|
+        analyzer_tables.update hotspot.generate_records(result_hash[hotspot.name])
       end
       analyzer_tables.generate_records
       rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables)

spec/metric_fu/metrics/reek/hotspot_spec.rb

@@ -0,0 +1,156 @@
+require 'spec_helper'
+MetricFu.metrics_require { 'hotspots/metric' }
+MetricFu.metrics_require { 'hotspots/hotspot' }
+MetricFu.metrics_require { 'hotspots/analysis/record' }
+MetricFu.metrics_require { 'reek/hotspot' }
+describe MetricFu::ReekHotspot do
+  let(:data) do
+    {
+      "metric"=>:reek,
+      "file_path"=>"lib/metric_fu.rb",
+      "reek__message"=>"doesn't depend on instance state",
+      "reek__type_name"=>"UtilityFunction",
+      "reek__value"=>nil,
+      "reek__value_description"=>nil,
+      "reek__comparable_message"=>"doesn't depend on instance state",
+      "class_name"=>"MetricFu",
+      "method_name"=>"MetricFu#current_time"
+    }
+  end
+  let(:row) do MetricFu::Record.new(data, :unused_variable) end
+  # TODO: This naming could be more clear
+  let(:analyzer) do MetricFu::ReekHotspot.new end
+  let(:tool_table) do
+    table = MetricFu::Table.new(:column_names => analyzer.columns)
+    table << row
+    table
+  end
+  let(:tool_tables) do {:reek => tool_table} end
+  let(:metric_violations) do tool_tables[analyzer.name] end
+
+  it "ranks and calculates reek hotspot scores" do
+    granularity = 'file_path'
+    metric_ranking = build_metric_ranking(metric_ranking, granularity)
+    test_metric_ranking(metric_ranking, granularity)
+
+    items_to_score = reek_items_to_score
+    master_ranking = build_master_ranking(items_to_score)
+    test_calculate_score(master_ranking, items_to_score)
+  end
+
+  def build_metric_ranking(metric_ranking, granularity)
+    metric_ranking = MetricFu::Ranking.new
+    metric_violations.each do |row|
+      location = row[granularity]
+      expect(location).to eq(data["file_path"])
+      metric_ranking[location] ||= []
+      mapped_row = analyzer.map(row)
+      expect(mapped_row).to eq(1) # present
+      metric_ranking[location] << mapped_row
+    end
+    metric_ranking
+  end
+  def test_metric_ranking(metric_ranking, granularity)
+    metric_ranking.each do |item, scores|
+      expect(item).to eq(data["file_path"])
+      expect(scores).to eq([1])
+      reduced_score = analyzer.reduce(scores)
+      expect(reduced_score).to eq(1) # sum
+      metric_ranking[item] = reduced_score
+    end
+  end
+  def build_master_ranking(items_to_score)
+    master_ranking = MetricFu::Ranking.new
+    items_to_score.each do |location, score|
+      master_ranking[location] = score
+    end
+    master_ranking
+  end
+  def test_calculate_score(master_ranking, items_to_score)
+    item = 'lib/metric_fu.rb'
+    sorted_items = master_ranking.send(:sorted_items)
+    index = sorted_items.index(item)
+    expect(index).to eq(21)
+    length = items_to_score.size
+    expect(length).to eq(45)
+
+    adjusted_index = index + 1
+    worse_item_count = length - adjusted_index
+
+    score = Float(worse_item_count) / length
+
+    expect(analyzer.score(master_ranking, item)).to eq(score) # percentile
+    expect(master_ranking.percentile(item)).to eq(score)
+    expect(MetricFu::HotspotScoringStrategies.percentile(master_ranking, item)).to eq(score)
+  end
+
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:58:in `calculate_metric_scores'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:46:in `calculate_score_for_granularity'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:41:in `block in calculate_scores_by_granularities'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:40:in `each'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:40:in `calculate_scores_by_granularities'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:18:in `block in calculate_scores'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:17:in `each'
+  # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:17:in `calculate_scores'
+  # lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb:57:in `setup'
+  # lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb:25:in `initialize'
+  # lib/metric_fu/metrics/hotspots/generator.rb:22:in `new'
+  # lib/metric_fu/metrics/hotspots/generator.rb:22:in `analyze'
+  # lib/metric_fu/generator.rb:107:in `generate_result'
+  # lib/metric_fu/reporting/result.rb:50:in `add'
+  # lib/metric_fu/run.rb:19:in `block in measure'
+  # lib/metric_fu/run.rb:17:in `each'
+  # lib/metric_fu/run.rb:17:in `measure'
+  # lib/metric_fu/run.rb:8:in `run'
+  # lib/metric_fu/cli/helper.rb:18:in `run'
+  # lib/metric_fu/cli/client.rb:18:in `run'
+  def reek_items_to_score
+    {
+      "lib/metric_fu.rb"=>4,
+      "lib/metric_fu/cli/client.rb"=>1,
+      "lib/metric_fu/cli/helper.rb"=>5,
+      "lib/metric_fu/cli/parser.rb"=>33,
+      "lib/metric_fu/configuration.rb"=>4,
+      "lib/metric_fu/constantize.rb"=>8,
+      "lib/metric_fu/data_structures/line_numbers.rb"=>11,
+      "lib/metric_fu/data_structures/location.rb"=>7,
+      "lib/metric_fu/data_structures/sexp_node.rb"=>5,
+      "lib/metric_fu/environment.rb"=>4,
+      "lib/metric_fu/errors/analysis_error.rb"=>1,
+      "lib/metric_fu/formatter.rb"=>1,
+      "lib/metric_fu/formatter/html.rb"=>11,
+      "lib/metric_fu/formatter/syntax.rb"=>2,
+      "lib/metric_fu/formatter/yaml.rb"=>1,
+      "lib/metric_fu/gem_run.rb"=>3,
+      "lib/metric_fu/gem_version.rb"=>5,
+      "lib/metric_fu/generator.rb"=>7,
+      "lib/metric_fu/io.rb"=>5,
+      "lib/metric_fu/loader.rb"=>6,
+      "lib/metric_fu/logger.rb"=>4,
+      "lib/metric_fu/logging/mf_debugger.rb"=>2,
+      "lib/metric_fu/metric.rb"=>7,
+      "lib/metric_fu/metrics/cane/generator.rb"=>6,
+      "lib/metric_fu/metrics/cane/grapher.rb"=>2,
+      "lib/metric_fu/metrics/cane/metric.rb"=>2,
+      "lib/metric_fu/metrics/cane/violations.rb"=>7,
+      "lib/metric_fu/metrics/churn/generator.rb"=>3,
+      "lib/metric_fu/metrics/churn/hotspot.rb"=>9,
+      "lib/metric_fu/metrics/churn/metric.rb"=>1,
+      "lib/metric_fu/metrics/flay/generator.rb"=>4,
+      "lib/metric_fu/metrics/flay/grapher.rb"=>2,
+      "lib/metric_fu/metrics/flay/hotspot.rb"=>5,
+      "lib/metric_fu/metrics/flay/metric.rb"=>1,
+      "lib/metric_fu/metrics/flog/generator.rb"=>11,
+      "lib/metric_fu/metrics/flog/grapher.rb"=>11,
+      "lib/metric_fu/metrics/flog/hotspot.rb"=>8,
+      "lib/metric_fu/metrics/flog/metric.rb"=>1,
+      "lib/metric_fu/metrics/hotspots/analysis/analyzed_problems.rb"=>1,
+      "lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb"=>8,
+      "lib/metric_fu/metrics/hotspots/analysis/grouping.rb"=>1,
+      "lib/metric_fu/metrics/hotspots/analysis/groupings.rb"=>1,
+      "lib/metric_fu/metrics/hotspots/analysis/problems.rb"=>2,
+      "lib/metric_fu/metrics/hotspots/analysis/ranked_problem_location.rb"=>5,
+      "lib/metric_fu/metrics/hotspots/analysis/ranking.rb"=>1,
+    }
+  end
+end