@@ -19,7 +19,6 @@ module Reek def initialize(outer, name, exp = nil) super(outer, name, exp) @superclass = exp[2] if exp - @parsed_methods = [] @instance_variables = Set.new end @@ -40,10 +39,6 @@ module Reek @instance_variables << Name.new(sym) end - def record_method(meth) - @parsed_methods << meth - end - def outer_name "#{@outer.outer_name}#{@name}#" end @@ -51,9 +46,5 @@ module Reek def variable_names @instance_variables end - - def parameterized_methods(min_clump_size) - parsed_methods.select {|meth| meth.parameters.length >= min_clump_size } - end end end lib/reek/class_context.rb @@ -17,7 +17,7 @@ module Reek @outer.outer_name end - def to_s + def to_s # SMELL: should be unnecessary :( @outer.to_s end end lib/reek/if_context.rb @@ -24,6 +24,7 @@ module Reek super(outer, exp) @name = name @attributes = Set.new + @parsed_methods = [] end def myself @@ -35,10 +36,18 @@ module Reek @myself.const_or_nil(modname.to_s) end + def parameterized_methods(min_clump_size) + @parsed_methods.select {|meth| meth.parameters.length >= min_clump_size } + end + def record_attribute(attr) @attributes << Name.new(attr) end + def record_method(meth) + @parsed_methods << meth + end + def check_for_attribute_declaration(exp) if [:attr, :attr_reader, :attr_writer, :attr_accessor].include? exp[2] exp[3][1..-1].each {|arg| record_attribute(arg[1])} lib/reek/module_context.rb @@ -20,7 +20,7 @@ module Reek class DataClump < SmellDetector def self.contexts # :nodoc: - [:class] + [:class, :module] end # The name of the config field that sets the maximum allowed @@ -44,14 +44,14 @@ module Reek end # - # Checks the given ClassContext for multiple identical conditional tests. + # Checks the given class or module for multiple identical parameter sets. # Remembers any smells found. # - def examine_context(klass) - max_copies = value(MAX_COPIES_KEY, klass, DEFAULT_MAX_COPIES) - min_clump_size = value(MIN_CLUMP_SIZE_KEY, klass, DEFAULT_MIN_CLUMP_SIZE) - MethodGroup.new(klass, min_clump_size, max_copies).clumps.each do |clump, occurs| - found(klass, "takes parameters #{DataClump.print_clump(clump)} to #{occurs} methods") + def examine_context(ctx) + max_copies = value(MAX_COPIES_KEY, ctx, DEFAULT_MAX_COPIES) + min_clump_size = value(MIN_CLUMP_SIZE_KEY, ctx, DEFAULT_MIN_CLUMP_SIZE) + MethodGroup.new(ctx, min_clump_size, max_copies).clumps.each do |clump, occurs| + found(ctx, "takes parameters #{DataClump.print_clump(clump)} to #{occurs} methods") end end @@ -68,15 +68,15 @@ module Reek methods.map {|meth| meth.parameters.names.sort}.intersection end - def initialize(klass, min_clump_size, max_copies) - @klass = klass + def initialize(ctx, min_clump_size, max_copies) + @ctx = ctx @min_clump_size = min_clump_size @max_copies = max_copies end def clumps results = Hash.new(0) - @klass.parameterized_methods(@min_clump_size).bounded_power_set(@max_copies).each do |methods| + @ctx.parameterized_methods(@min_clump_size).bounded_power_set(@max_copies).each do |methods| clump = MethodGroup.intersection_of_parameters_of(methods) if clump.length >= @min_clump_size results[clump] = [methods.length, results[clump]].max lib/reek/smells/data_clump.rb @@ -4,10 +4,10 @@ require 'reek/smells/data_clump' include Reek::Smells -describe DataClump do - it 'should not report small parameter sets' do +shared_examples_for 'a data clump detector' do + it 'does not report small parameter sets' do src = <<EOS -class Scrunch +#{@context} Scrunch def first(pa) @field == :sym ? 0 : 3; end def second(pa) @field == :sym; end def third(pa) pa - pb + @fred; end @@ -17,9 +17,9 @@ EOS src.should_not reek end - it 'should report 3 identical pairs in a class' do + it 'reports 3 identical pairs in a class' do src = <<EOS -class Scrunch +#{@context} Scrunch def first(pa, pb) @field == :sym ? 0 : 3; end def second(pa, pb) @field == :sym; end def third(pa, pb) pa - pb + @fred; end @@ -29,9 +29,9 @@ EOS src.should reek_of(:DataClump, /\[pa, pb\]/, /3 methods/) end - it 'should report 3 swapped pairs in a class' do + it 'reports 3 swapped pairs in a class' do src = <<EOS -class Scrunch +#{@context} Scrunch def one(pa, pb) @field == :sym ? 0 : 3; end def two(pb, pa) @field == :sym; end def tri(pa, pb) pa - pb + @fred; end @@ -41,9 +41,9 @@ EOS src.should reek_of(:DataClump, /\[pa, pb\]/, /3 methods/) end - it 'should report 3 identical parameter sets in a class' do + it 'reports 3 identical parameter sets in a class' do src = <<EOS -class Scrunch +#{@context} Scrunch def first(pa, pb, pc) @field == :sym ? 0 : 3; end def second(pa, pb, pc) @field == :sym; end def third(pa, pb, pc) pa - pb + @fred; end @@ -56,9 +56,9 @@ EOS src.should_not reek_of(:DataClump, /\[pb, pc\]/, /3 methods/) end - it 'should recognise re-ordered identical parameter sets' do + it 'recognises re-ordered identical parameter sets' do src = <<EOS -class Scrunch +#{@context} Scrunch def first(pb, pa, pc) @field == :sym ? 0 : 3; end def second(pc, pb, pa) @field == :sym; end def third(pa, pb, pc) pa - pb + @fred; end @@ -71,9 +71,9 @@ EOS src.should_not reek_of(:DataClump, /\[pb, pc\]/, /3 methods/) end - it 'should count only identical parameter sets' do + it 'counts only identical parameter sets' do src = <<EOS -class RedCloth +#{@context} RedCloth def fa(p1, p2, p3, conten) end def fb(p1, p2, p3, conten) end def fc(name, windowW, windowH) end @@ -82,6 +82,24 @@ EOS src.should_not reek_of(:DataClump) end +end + +describe DataClump do + context 'in a class' do + before :each do + @context = 'class' + end + + it_should_behave_like 'a data clump detector' + end + + context 'in a module' do + before :each do + @context = 'module' + end + + it_should_behave_like 'a data clump detector' + end # TODO: include singleton methods in the calcs end spec/reek/smells/data_clump_spec.rb d543c7a35483adefa720aad89e4e6d0debb8215e Kevin Rutherford kevin@rutherford-software.com http://github.com/kevinrutherford/reek/commit/6e84f60540d4fe4f14acd6ee4339f75d1be5c3be 6e84f60540d4fe4f14acd6ee4339f75d1be5c3be 2009-11-02T06:37:44-08:00 2009-11-02T06:37:44-08:00 Expanded method collection to modules 8d779e97ea5ce2eac672c397448d9837415fa29c Kevin Rutherford kevin@rutherford-software.com