TwitterClient::RealTwitterClient#update_status_for has approx 7 statements Open
def update_status_for(user)
- 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.)
TwitterClient::TwitterStatus tests '@status.nil?' at least 4 times Open
return nil if @status.nil?
@status.id
end
def posted_at
- Read upRead up
- Exclude checks
Repeated Conditional
is a special case of Simulated Polymorphism
. Basically it means you are checking the same value throughout a single class and take decisions based on this.
Example
Given
class RepeatedConditionals
attr_accessor :switch
def repeat_1
puts "Repeat 1!" if switch
end
def repeat_2
puts "Repeat 2!" if switch
end
def repeat_3
puts "Repeat 3!" if switch
end
end
Reek would emit the following warning:
test.rb -- 4 warnings:
[5, 9, 13]:RepeatedConditionals tests switch at least 3 times (RepeatedConditional)
If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction.
TwitterClient::NilTwitterClient has no descriptive comment Open
class NilTwitterClient
- 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
TwitterClient has no descriptive comment Open
module TwitterClient
- 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
TwitterClient::TwitterStatus#== calls 'other_status.nil?' 2 times Open
return true if @status.nil? && other_status.nil?
return false if @status.nil? || other_status.nil?
- Read upRead up
- Exclude checks
Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.
Reek implements a check for Duplicate Method Call.
Example
Here's a very much simplified and contrived example. The following method will report a warning:
def double_thing()
@other.thing + @other.thing
end
One quick approach to silence Reek would be to refactor the code thus:
def double_thing()
thing = @other.thing
thing + thing
end
A slightly different approach would be to replace all calls of double_thing
by calls to @other.double_thing
:
class Other
def double_thing()
thing + thing
end
end
The approach you take will depend on balancing other factors in your code.
TwitterClient::TwitterStatus#== calls '@status.nil?' 2 times Open
return true if @status.nil? && other_status.nil?
return false if @status.nil? || other_status.nil?
- Read upRead up
- Exclude checks
Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.
Reek implements a check for Duplicate Method Call.
Example
Here's a very much simplified and contrived example. The following method will report a warning:
def double_thing()
@other.thing + @other.thing
end
One quick approach to silence Reek would be to refactor the code thus:
def double_thing()
thing = @other.thing
thing + thing
end
A slightly different approach would be to replace all calls of double_thing
by calls to @other.double_thing
:
class Other
def double_thing()
thing + thing
end
end
The approach you take will depend on balancing other factors in your code.
TwitterClient::RealTwitterClient has no descriptive comment Open
class RealTwitterClient
- 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
TwitterClient::TwitterStatus has no descriptive comment Open
class TwitterStatus
- 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
TwitterClient::TwitterStatus#id performs a nil-check Open
return nil if @status.nil?
- Read upRead up
- Exclude checks
A NilCheck
is a type check. Failures of NilCheck
violate the "tell, don't ask" principle.
Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
Example
Given
class Klass
def nil_checker(argument)
if argument.nil?
puts "argument isn't nil!"
end
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[3]:Klass#nil_checker performs a nil-check. (NilCheck)
TwitterClient::TwitterStatus#text performs a nil-check Open
return '' if @status.nil?
- Read upRead up
- Exclude checks
A NilCheck
is a type check. Failures of NilCheck
violate the "tell, don't ask" principle.
Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
Example
Given
class Klass
def nil_checker(argument)
if argument.nil?
puts "argument isn't nil!"
end
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[3]:Klass#nil_checker performs a nil-check. (NilCheck)
TwitterClient::TwitterStatus#== performs a nil-check Open
return true if @status.nil? && other_status.nil?
return false if @status.nil? || other_status.nil?
- Read upRead up
- Exclude checks
A NilCheck
is a type check. Failures of NilCheck
violate the "tell, don't ask" principle.
Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
Example
Given
class Klass
def nil_checker(argument)
if argument.nil?
puts "argument isn't nil!"
end
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[3]:Klass#nil_checker performs a nil-check. (NilCheck)
TwitterClient::TwitterStatus#to_s performs a nil-check Open
return '' if @status.nil?
- Read upRead up
- Exclude checks
A NilCheck
is a type check. Failures of NilCheck
violate the "tell, don't ask" principle.
Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
Example
Given
class Klass
def nil_checker(argument)
if argument.nil?
puts "argument isn't nil!"
end
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[3]:Klass#nil_checker performs a nil-check. (NilCheck)
TwitterClient::TwitterStatus#posted_at performs a nil-check Open
return nil if @status.nil?
- Read upRead up
- Exclude checks
A NilCheck
is a type check. Failures of NilCheck
violate the "tell, don't ask" principle.
Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.
Example
Given
class Klass
def nil_checker(argument)
if argument.nil?
puts "argument isn't nil!"
end
end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[3]:Klass#nil_checker performs a nil-check. (NilCheck)
TwitterClient::RealTwitterClient#strip_at_sign doesn't depend on instance state (maybe move it to another class?) Open
def strip_at_sign(handle)
- Read upRead up
- Exclude checks
A Utility Function is any instance method that has no dependency on the state of the instance.
TwitterClient::NilTwitterClient#valid_twitter_user? has unused parameter 'user' Open
def valid_twitter_user?(user)
- Read upRead up
- Exclude checks
Unused Parameter
refers to methods with parameters that are unused in scope of the method.
Having unused parameters in a method is code smell because leaving dead code in a method can never improve the method and it makes the code confusing to read.
Example
Given:
class Klass
def unused_parameters(x,y,z)
puts x,y # but not z
end
end
Reek would emit the following warning:
[2]:Klass#unused_parameters has unused parameter 'z' (UnusedParameters)
TwitterClient::NilTwitterClient#update_status_for has unused parameter 'user' Open
def update_status_for(user)
- Read upRead up
- Exclude checks
Unused Parameter
refers to methods with parameters that are unused in scope of the method.
Having unused parameters in a method is code smell because leaving dead code in a method can never improve the method and it makes the code confusing to read.
Example
Given:
class Klass
def unused_parameters(x,y,z)
puts x,y # but not z
end
end
Reek would emit the following warning:
[2]:Klass#unused_parameters has unused parameter 'z' (UnusedParameters)
TwitterClient::NilTwitterClient#last_status_for has unused parameter 'user' Open
def last_status_for(user)
- Read upRead up
- Exclude checks
Unused Parameter
refers to methods with parameters that are unused in scope of the method.
Having unused parameters in a method is code smell because leaving dead code in a method can never improve the method and it makes the code confusing to read.
Example
Given:
class Klass
def unused_parameters(x,y,z)
puts x,y # but not z
end
end
Reek would emit the following warning:
[2]:Klass#unused_parameters has unused parameter 'z' (UnusedParameters)
TODO found Open
# TODO: I'm sure there's a more efficient way to do this
- Exclude checks
Unused method argument - user
. If it's necessary, use _
or _user
as an argument name to indicate that it won't be used. You can also write as valid_twitter_user?(*)
if you want the method to accept any arguments but don't care about them. Open
def valid_twitter_user?(user)
- Read upRead up
- Exclude checks
This cop checks for unused method arguments.
Example:
# bad
def some_method(used, unused, _unused_but_allowed)
puts used
end
Example:
# good
def some_method(used, _unused, _unused_but_allowed)
puts used
end
Unused method argument - user
. If it's necessary, use _
or _user
as an argument name to indicate that it won't be used. You can also write as last_status_for(*)
if you want the method to accept any arguments but don't care about them. Open
def last_status_for(user)
- Read upRead up
- Exclude checks
This cop checks for unused method arguments.
Example:
# bad
def some_method(used, unused, _unused_but_allowed)
puts used
end
Example:
# good
def some_method(used, _unused, _unused_but_allowed)
puts used
end
Useless private
access modifier. Open
private
- Read upRead up
- Exclude checks
This cop checks for redundant access modifiers, including those with no
code, those which are repeated, and leading public
modifiers in a
class or module body. Conditionally-defined methods are considered as
always being defined, and thus access modifiers guarding such methods
are not redundant.
Example:
class Foo
public # this is redundant (default access is public)
def method
end
private # this is not redundant (a method is defined)
def method2
end
private # this is redundant (no following methods are defined)
end
Example:
class Foo
# The following is not redundant (conditionally defined methods are
# considered as always defining a method)
private
if condition?
def method
end
end
protected # this is not redundant (method is defined)
define_method(:method2) do
end
protected # this is redundant (repeated from previous modifier)
[1,2,3].each do |i|
define_method("foo#{i}") do
end
end
# The following is redundant (methods defined on the class'
# singleton class are not affected by the public modifier)
public
def self.method3
end
end
Example:
# Lint/UselessAccessModifier:
# ContextCreatingMethods:
# - concerning
require 'active_support/concern'
class Foo
concerning :Bar do
def some_public_method
end
private
def some_private_method
end
end
# this is not redundant because `concerning` created its own context
private
def some_other_private_method
end
end
Example:
# Lint/UselessAccessModifier:
# MethodCreatingMethods:
# - delegate
require 'active_support/core_ext/module/delegation'
class Foo
# this is not redundant because `delegate` creates methods
private
delegate :method_a, to: :method_b
end