Class has too many lines. [145/100] Open
class Options < OpenStruct
include Enumerable
include Validation::Validate
def keys
- Read upRead up
- Exclude checks
This cop checks if the length a class exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Method has too many lines. [20/10] Open
def reload!
conf = {}
@config_sources.each do |source|
source_conf = source.load
- Read upRead up
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Method __convert
has a Cognitive Complexity of 17 (exceeds 5 allowed). Consider refactoring. Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method has too many lines. [12/10] Open
def merge!(hash)
current = to_hash
DeepMerge.deep_merge!(
hash.dup,
current,
- Read upRead up
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Assignment Branch Condition size for __convert is too high. [17.8/15] Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric
Method has too many lines. [11/10] Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Method has too many lines. [11/10] Open
def to_hash
result = {}
marshal_dump.each do |k, v|
if v.instance_of? Config::Options
result[k] = v.to_hash
- Read upRead up
- Exclude checks
This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.
Cyclomatic complexity for __convert is too high. [7/6] Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
This cop checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.
An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one.
Perceived complexity for __convert is too high. [8/7] Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
This cop tries to produce a complexity score that's a measure of the
complexity the reader experiences when looking at a method. For that
reason it considers when
nodes as something that doesn't add as much
complexity as an if
or a &&
. Except if it's one of those special
case
/when
constructs where there's no expression after case
. Then
the cop treats it as an if
/elsif
/elsif
... and lets all the when
nodes count. In contrast to the CyclomaticComplexity cop, this cop
considers else
nodes as adding complexity.
Example:
def my_method # 1
if cond # 1
case var # 2 (0.8 + 4 * 0.2, rounded)
when 1 then func_one
when 2 then func_two
when 3 then func_three
when 4..10 then func_other
end
else # 1
do_something until a && b # 2
end # ===
end # 7 complexity points
Complex method Config::Options#__convert (26.1) Open
def __convert(h) #:nodoc:
s = self.class.new
h.each do |k, v|
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
Flog calculates the ABC score for methods. The ABC score is based on assignments, branches (method calls), and conditions.
You can read more about ABC metrics or the flog tool
Config::Options#descend_array refers to 'value' more than self (maybe move it to another class?) Open
if value.instance_of? Config::Options
value.to_hash
elsif value.instance_of? Array
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options#add_source! refers to 'source' more than self (maybe move it to another class?) Open
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options#__convert has approx 10 statements Open
def __convert(h) #:nodoc:
- Read upRead up
- Exclude checks
A method with Too Many Statements
is any method that has a large number of lines.
Too Many Statements
warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements
counts +1 for every simple statement in a method and +1 for every statement within a control structure (if
, else
, case
, when
, for
, while
, until
, begin
, rescue
) but it doesn't count the control structure itself.
So the following method would score +6 in Reek's statement-counting algorithm:
def parse(arg, argv, &error)
if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
return nil, block, nil # +1
end
opt = (val = parse_arg(val, &error))[1] # +2
val = conv_arg(*val) # +3
if opt and !arg
argv.shift # +4
else
val[0] = nil # +5
end
val # +6
end
(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)
Config::Options#to_hash has approx 6 statements Open
def to_hash
- Read upRead up
- Exclude checks
A method with Too Many Statements
is any method that has a large number of lines.
Too Many Statements
warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements
counts +1 for every simple statement in a method and +1 for every statement within a control structure (if
, else
, case
, when
, for
, while
, until
, begin
, rescue
) but it doesn't count the control structure itself.
So the following method would score +6 in Reek's statement-counting algorithm:
def parse(arg, argv, &error)
if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
return nil, block, nil # +1
end
opt = (val = parse_arg(val, &error))[1] # +2
val = conv_arg(*val) # +3
if opt and !arg
argv.shift # +4
else
val[0] = nil # +5
end
val # +6
end
(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)
Config::Options#to_hash refers to 'result' more than self (maybe move it to another class?) Open
result[k] = v.to_hash
elsif v.instance_of? Array
result[k] = descend_array(v)
else
result[k] = v
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options#to_hash refers to 'v' more than self (maybe move it to another class?) Open
if v.instance_of? Config::Options
result[k] = v.to_hash
elsif v.instance_of? Array
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options#__convert contains iterators nested 2 deep Open
v = v.collect { |e| e.instance_of?(Hash) ? __convert(e) : e }
- Read upRead up
- Exclude checks
A Nested Iterator
occurs when a block contains another block.
Example
Given
class Duck
class << self
def duck_names
%i!tick trick track!.each do |surname|
%i!duck!.each do |last_name|
puts "full name is #{surname} #{last_name}"
end
end
end
end
end
Reek would report the following warning:
test.rb -- 1 warning:
[5]:Duck#duck_names contains iterators nested 2 deep (NestedIterators)
Config::Options has at least 19 methods Open
class Options < OpenStruct
- Read upRead up
- Exclude checks
Too Many Methods
is a special case of LargeClass
.
Example
Given this configuration
TooManyMethods:
max_methods: 3
and this code:
class TooManyMethods
def one; end
def two; end
def three; end
def four; end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:TooManyMethods has at least 4 methods (TooManyMethods)
Config::Options#prepend_source! refers to 'source' more than self (maybe move it to another class?) Open
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options#reload! has approx 8 statements Open
def reload!
- Read upRead up
- Exclude checks
A method with Too Many Statements
is any method that has a large number of lines.
Too Many Statements
warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements
counts +1 for every simple statement in a method and +1 for every statement within a control structure (if
, else
, case
, when
, for
, while
, until
, begin
, rescue
) but it doesn't count the control structure itself.
So the following method would score +6 in Reek's statement-counting algorithm:
def parse(arg, argv, &error)
if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
return nil, block, nil # +1
end
opt = (val = parse_arg(val, &error))[1] # +2
val = conv_arg(*val) # +3
if opt and !arg
argv.shift # +4
else
val[0] = nil # +5
end
val # +6
end
(You might argue that the two assigments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)
Config::Options#__convert refers to 'v' more than self (maybe move it to another class?) Open
if v.is_a?(Hash)
v = v["type"] == "hash" ? v["contents"] : __convert(v)
elsif v.is_a?(Array)
v = v.collect { |e| e.instance_of?(Hash) ? __convert(e) : e }
- Read upRead up
- Exclude checks
Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.
Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.
Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.
Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.
Example
Running Reek on:
class Warehouse
def sale_price(item)
(item.price - item.rebate) * @vat
end
end
would report:
Warehouse#total_price refers to item more than self (FeatureEnvy)
since this:
(item.price - item.rebate)
belongs to the Item class, not the Warehouse.
Config::Options assumes too much for instance variable '@config_sources' Open
class Options < OpenStruct
- Read upRead up
- Exclude checks
Classes should not assume that instance variables are set or present outside of the current class definition.
Good:
class Foo
def initialize
@bar = :foo
end
def foo?
@bar == :foo
end
end
Good as well:
class Foo
def foo?
bar == :foo
end
def bar
@bar ||= :foo
end
end
Bad:
class Foo
def go_foo!
@bar = :foo
end
def foo?
@bar == :foo
end
end
Example
Running Reek on:
class Dummy
def test
@ivar
end
end
would report:
[1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar
Note that this example would trigger this smell warning as well:
class Parent
def initialize(omg)
@omg = omg
end
end
class Child < Parent
def foo
@omg
end
end
The way to address the smell warning is that you should create an attr_reader
to use @omg
in the subclass and not access @omg
directly like this:
class Parent
attr_reader :omg
def initialize(omg)
@omg = omg
end
end
class Child < Parent
def foo
omg
end
end
Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.
If you don't want to expose those methods as public API just make them private like this:
class Parent
def initialize(omg)
@omg = omg
end
private
attr_reader :omg
end
class Child < Parent
def foo
omg
end
end
Current Support in Reek
An instance variable must:
- be set in the constructor
- or be accessed through a method with lazy initialization / memoization.
If not, Instance Variable Assumption will be reported.
Config::Options#__convert manually dispatches method call Open
k = k.to_s if !k.respond_to?(:to_sym) && k.respond_to?(:to_s)
- Read upRead up
- Exclude checks
Reek reports a Manual Dispatch smell if it finds source code that manually checks whether an object responds to a method before that method is called. Manual dispatch is a type of Simulated Polymorphism which leads to code that is harder to reason about, debug, and refactor.
Example
class MyManualDispatcher
attr_reader :foo
def initialize(foo)
@foo = foo
end
def call
foo.bar if foo.respond_to?(:bar)
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[9]: MyManualDispatcher manually dispatches method call (ManualDispatch)
Config::Options assumes too much for instance variable '@table' Open
class Options < OpenStruct
- Read upRead up
- Exclude checks
Classes should not assume that instance variables are set or present outside of the current class definition.
Good:
class Foo
def initialize
@bar = :foo
end
def foo?
@bar == :foo
end
end
Good as well:
class Foo
def foo?
bar == :foo
end
def bar
@bar ||= :foo
end
end
Bad:
class Foo
def go_foo!
@bar = :foo
end
def foo?
@bar == :foo
end
end
Example
Running Reek on:
class Dummy
def test
@ivar
end
end
would report:
[1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar
Note that this example would trigger this smell warning as well:
class Parent
def initialize(omg)
@omg = omg
end
end
class Child < Parent
def foo
@omg
end
end
The way to address the smell warning is that you should create an attr_reader
to use @omg
in the subclass and not access @omg
directly like this:
class Parent
attr_reader :omg
def initialize(omg)
@omg = omg
end
end
class Child < Parent
def foo
omg
end
end
Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.
If you don't want to expose those methods as public API just make them private like this:
class Parent
def initialize(omg)
@omg = omg
end
private
attr_reader :omg
end
class Child < Parent
def foo
omg
end
end
Current Support in Reek
An instance variable must:
- be set in the constructor
- or be accessed through a method with lazy initialization / memoization.
If not, Instance Variable Assumption will be reported.
Config::Options has no descriptive comment Open
class Options < OpenStruct
- Read upRead up
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
Config::Options has missing safe method 'merge!' Open
def merge!(hash)
- Read upRead up
- Exclude checks
A candidate method for the Missing Safe Method
smell are methods whose names end with an exclamation mark.
An exclamation mark in method names means (the explanation below is taken from here ):
The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.
Such a method is called Missing Safe Method
if and only if her non-bang version does not exist and this method is reported as a smell.
Example
Given
class C
def foo; end
def foo!; end
def bar!; end
end
Reek would report bar!
as Missing Safe Method
smell but not foo!
.
Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:
class Parent
def foo; end
end
module Dangerous
def foo!; end
end
class Son < Parent
include Dangerous
end
class Daughter < Parent
end
In this example, Reek would not report the Missing Safe Method
smell for the method foo
of the Dangerous
module.
Config::Options has missing safe method 'add_source!' Open
def add_source!(source)
- Read upRead up
- Exclude checks
A candidate method for the Missing Safe Method
smell are methods whose names end with an exclamation mark.
An exclamation mark in method names means (the explanation below is taken from here ):
The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.
Such a method is called Missing Safe Method
if and only if her non-bang version does not exist and this method is reported as a smell.
Example
Given
class C
def foo; end
def foo!; end
def bar!; end
end
Reek would report bar!
as Missing Safe Method
smell but not foo!
.
Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:
class Parent
def foo; end
end
module Dangerous
def foo!; end
end
class Son < Parent
include Dangerous
end
class Daughter < Parent
end
In this example, Reek would not report the Missing Safe Method
smell for the method foo
of the Dangerous
module.
Config::Options has missing safe method 'reload!' Open
def reload!
- Read upRead up
- Exclude checks
A candidate method for the Missing Safe Method
smell are methods whose names end with an exclamation mark.
An exclamation mark in method names means (the explanation below is taken from here ):
The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.
Such a method is called Missing Safe Method
if and only if her non-bang version does not exist and this method is reported as a smell.
Example
Given
class C
def foo; end
def foo!; end
def bar!; end
end
Reek would report bar!
as Missing Safe Method
smell but not foo!
.
Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:
class Parent
def foo; end
end
module Dangerous
def foo!; end
end
class Son < Parent
include Dangerous
end
class Daughter < Parent
end
In this example, Reek would not report the Missing Safe Method
smell for the method foo
of the Dangerous
module.
Config::Options has missing safe method 'prepend_source!' Open
def prepend_source!(source)
- Read upRead up
- Exclude checks
A candidate method for the Missing Safe Method
smell are methods whose names end with an exclamation mark.
An exclamation mark in method names means (the explanation below is taken from here ):
The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.
Such a method is called Missing Safe Method
if and only if her non-bang version does not exist and this method is reported as a smell.
Example
Given
class C
def foo; end
def foo!; end
def bar!; end
end
Reek would report bar!
as Missing Safe Method
smell but not foo!
.
Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:
class Parent
def foo; end
end
module Dangerous
def foo!; end
end
class Son < Parent
include Dangerous
end
class Daughter < Parent
end
In this example, Reek would not report the Missing Safe Method
smell for the method foo
of the Dangerous
module.
Config::Options#__convert has the variable name 'e' Open
v = v.collect { |e| e.instance_of?(Hash) ? __convert(e) : e }
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#__convert has the variable name 'k' Open
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#__convert has the variable name 's' Open
s = self.class.new
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#to_hash has the variable name 'k' Open
marshal_dump.each do |k, v|
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#__convert has the parameter name 'h' Open
def __convert(h) #:nodoc:
- Read upRead up
- Exclude checks
An Uncommunicative Parameter Name
is a parameter name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#__convert has the variable name 'v' Open
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Config::Options#to_hash has the variable name 'v' Open
marshal_dump.each do |k, v|
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Similar blocks of code found in 2 locations. Consider refactoring. Open
def prepend_source!(source)
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
@config_sources ||= []
- Read upRead up
Duplicated Code
Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).
Tuning
This issue has a mass of 31.
We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.
The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.
If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.
See codeclimate-duplication
's documentation for more information about tuning the mass threshold in your .codeclimate.yml
.
Refactorings
- Extract Method
- Extract Class
- Form Template Method
- Introduce Null Object
- Pull Up Method
- Pull Up Field
- Substitute Algorithm
Further Reading
- Don't Repeat Yourself on the C2 Wiki
- Duplicated Code on SourceMaking
- Refactoring: Improving the Design of Existing Code by Martin Fowler. Duplicated Code, p76
Similar blocks of code found in 2 locations. Consider refactoring. Open
def add_source!(source)
# handle yaml file paths
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
- Read upRead up
Duplicated Code
Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).
Tuning
This issue has a mass of 31.
We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.
The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.
If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.
See codeclimate-duplication
's documentation for more information about tuning the mass threshold in your .codeclimate.yml
.
Refactorings
- Extract Method
- Extract Class
- Form Template Method
- Introduce Null Object
- Pull Up Method
- Pull Up Field
- Substitute Algorithm
Further Reading
- Don't Repeat Yourself on the C2 Wiki
- Duplicated Code on SourceMaking
- Refactoring: Improving the Design of Existing Code by Martin Fowler. Duplicated Code, p76
Use alias load! reload!
instead of alias :load! :reload!
. Open
alias :load! :reload!
- Read upRead up
- Exclude checks
This cop enforces the use of either #alias
or #alias_method
depending on configuration.
It also flags uses of alias :symbol
rather than alias bareword
.
Example: EnforcedStyle: prefer_alias (default)
# bad
alias_method :bar, :foo
alias :bar :foo
# good
alias bar foo
Example: EnforcedStyle: preferaliasmethod
# bad
alias :bar :foo
alias bar foo
# good
alias_method :bar, :foo
Use alias to_h to_hash
instead of alias :to_h :to_hash
. Open
alias :to_h :to_hash
- Read upRead up
- Exclude checks
This cop enforces the use of either #alias
or #alias_method
depending on configuration.
It also flags uses of alias :symbol
rather than alias bareword
.
Example: EnforcedStyle: prefer_alias (default)
# bad
alias_method :bar, :foo
alias :bar :foo
# good
alias bar foo
Example: EnforcedStyle: preferaliasmethod
# bad
alias :bar :foo
alias bar foo
# good
alias_method :bar, :foo
Indent the first parameter one step more than the start of the previous line. Open
source_conf,
- Read upRead up
- Exclude checks
This cop checks the indentation of the first parameter in a method call. Parameters after the first one are checked by Style/AlignParameters, not by this cop.
Example:
# bad
some_method(
first_param,
second_param)
# good
some_method(
first_param,
second_param)
Indent the first parameter one step more than the start of the previous line. Open
hash.dup,
- Read upRead up
- Exclude checks
This cop checks the indentation of the first parameter in a method call. Parameters after the first one are checked by Style/AlignParameters, not by this cop.
Example:
# bad
some_method(
first_param,
second_param)
# good
some_method(
first_param,
second_param)
Indent )
the same as the start of the line where (
is. Open
)
- Read upRead up
- Exclude checks
This cops checks the indentation of hanging closing parentheses in
method calls, method definitions, and grouped expressions. A hanging
closing parenthesis means )
preceded by a line break.
Example:
# good: when x is on its own line, indent this way
func(
x,
y
)
# good: when x follows opening parenthesis, align parentheses
a = b * (x +
y
)
# bad
def func(
x,
y
)
end
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
require "json" unless defined?(JSON)
- Read upRead up
- Exclude checks
Checks if uses of quotes match the configured preference.
Example: EnforcedStyle: single_quotes (default)
# bad
"No special symbols"
"No string interpolation"
"Just text"
# good
'No special symbols'
'No string interpolation'
'Just text'
"Wait! What's #{this}!"
Example: EnforcedStyle: double_quotes
# bad
'Just some text'
'No special chars or interpolation'
# good
"Just some text"
"No special chars or interpolation"
"Every string in #{project} uses double_quotes"
Missing top-level class documentation comment. Open
class Options < OpenStruct
- Read upRead up
- Exclude checks
This cop checks for missing top-level documentation of classes and modules. Classes with no body are exempt from the check and so are namespace modules - modules that have nothing in their bodies except classes, other modules, or constant definitions.
The documentation requirement is annulled if the class or module has a "#:nodoc:" comment next to it. Likewise, "#:nodoc: all" does the same for all its children.
Example:
# bad
class Person
# ...
end
# good
# Description/Explanation of Person class
class Person
# ...
end
Indent )
the same as the start of the line where (
is. Open
)
- Read upRead up
- Exclude checks
This cops checks the indentation of hanging closing parentheses in
method calls, method definitions, and grouped expressions. A hanging
closing parenthesis means )
preceded by a line break.
Example:
# good: when x is on its own line, indent this way
func(
x,
y
)
# good: when x follows opening parenthesis, align parentheses
a = b * (x +
y
)
# bad
def func(
x,
y
)
end
Don't use parentheses around a method call. Open
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
v = v["type"] == "hash" ? v["contents"] : __convert(v)
- Read upRead up
- Exclude checks
Checks if uses of quotes match the configured preference.
Example: EnforcedStyle: single_quotes (default)
# bad
"No special symbols"
"No string interpolation"
"Just text"
# good
'No special symbols'
'No string interpolation'
'Just text'
"Wait! What's #{this}!"
Example: EnforcedStyle: double_quotes
# bad
'Just some text'
'No special chars or interpolation'
# good
"Just some text"
"No special chars or interpolation"
"Every string in #{project} uses double_quotes"
Don't use parentheses around a method call. Open
source = (Sources::YAMLSource.new(source)) if source.is_a?(String) || source.is_a?(Pathname)
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
v = v["type"] == "hash" ? v["contents"] : __convert(v)
- Read upRead up
- Exclude checks
Checks if uses of quotes match the configured preference.
Example: EnforcedStyle: single_quotes (default)
# bad
"No special symbols"
"No string interpolation"
"Just text"
# good
'No special symbols'
'No string interpolation'
'Just text'
"Wait! What's #{this}!"
Example: EnforcedStyle: double_quotes
# bad
'Just some text'
'No special chars or interpolation'
# good
"Just some text"
"No special chars or interpolation"
"Every string in #{project} uses double_quotes"
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
v = v["type"] == "hash" ? v["contents"] : __convert(v)
- Read upRead up
- Exclude checks
Checks if uses of quotes match the configured preference.
Example: EnforcedStyle: single_quotes (default)
# bad
"No special symbols"
"No string interpolation"
"Just text"
# good
'No special symbols'
'No string interpolation'
'Just text'
"Wait! What's #{this}!"
Example: EnforcedStyle: double_quotes
# bad
'Just some text'
'No special chars or interpolation'
# good
"Just some text"
"No special chars or interpolation"
"Every string in #{project} uses double_quotes"
Prefer to_s
over string interpolation. Open
public_send("#{param}")
- Read upRead up
- Exclude checks
This cop checks for strings that are just an interpolated expression.
Example:
# bad
"#{@var}"
# good
@var.to_s
# good if @var is already a String
@var
Use the return of the conditional for variable assignment and comparison. Open
if v.instance_of? Config::Options
result[k] = v.to_hash
elsif v.instance_of? Array
result[k] = descend_array(v)
else
- Exclude checks
Rename has_key?
to key?
. Open
def has_key?(key)
- Read upRead up
- Exclude checks
This cop makes sure that predicates are named properly.
Example:
# bad
def is_even?(value)
end
# good
def even?(value)
end
# bad
def has_value?
end
# good
def value?
end
Don't use parentheses around a method call. Open
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Use Hash#key?
instead of Hash#has_key?
. Open
@table.has_key?(key)
- Read upRead up
- Exclude checks
This cop (by default) checks for uses of methods Hash#haskey? and
Hash#hasvalue? where it enforces Hash#key? and Hash#value?
It is configurable to enforce the inverse, using verbose
method
names also.
Example: EnforcedStyle: short (default)
# bad Hash#haskey? Hash#hasvalue?
# good Hash#key? Hash#value?
Example: EnforcedStyle: verbose
# bad Hash#key? Hash#value?
# good Hash#haskey? Hash#hasvalue?
Don't use parentheses around a method call. Open
source = (Sources::HashSource.new(source)) if source.is_a?(Hash)
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?