Method has too many lines. [13/10] Open
def create
if ( is_employee? )
@notification = Notification.new(notification_params)
@notification.employee_id = @current_user.id
@notification.notification_date = @notification.get_date
- 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 update
if ( is_employee? )
@notification = Notification.find(params[:id])
if ( @notification.update(notification_params) )
flash[:notice] = "A notificação foi alterada com sucesso."
- 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.
NotificationsController#create has approx 7 statements Open
def create
- 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.)
NotificationsController tests '( is_employee? )' at least 5 times Open
if ( is_employee? )
@notification = Notification.new
else
redirect_to "/errors/error_500"
end
- 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.
NotificationsController assumes too much for instance variable '@current_user' Open
class NotificationsController < ApplicationController
- 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.
NotificationsController assumes too much for instance variable '@notification' Open
class NotificationsController < ApplicationController
- 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.
NotificationsController#show calls '@notification.employee_id' 2 times Open
@assignee = Employee.exists?(@notification.employee_id) ? Employee.find_by_id(@notification.employee_id).name : "Desconhecido"
- 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.
Keep a blank line before and after private
. Open
private
- Read upRead up
- Exclude checks
Access modifiers should be surrounded by blank lines.
Example:
# bad
class Foo
def bar; end
private
def baz; end
end
# good
class Foo
def bar; end
private
def baz; end
end
Space inside parentheses detected. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
if ( @notification.update(notification_params) )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around a method call. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Don't use parentheses around a method call. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
redirect_to "/errors/error_500"
- 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
flash[:alert] = "Notificação excluída com sucesso"
- 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 the condition of an if
. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Don't use parentheses around the condition of an if
. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Space inside parentheses detected. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around a method call. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Don't use parentheses around a method call. Open
if (@notification.save)
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Space inside parentheses detected. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Line is too long. [88/80] Open
# Description: Controller used to communicate with the proprietary view of notifications
- Exclude checks
Don't use parentheses around the condition of an if
. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Don't use parentheses around the condition of an if
. Open
if ( @notification.update(notification_params) )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Don't use parentheses around the condition of an if
. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
flash[:notice] = "A notificação foi alterada com sucesso."
- 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"
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
redirect_to "/errors/error_500"
- 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"
Line is too long. [132/80] Open
@assignee = Employee.exists?(@notification.employee_id) ? Employee.find_by_id(@notification.employee_id).name : "Desconhecido"
- Exclude checks
Don't use parentheses around the condition of an if
. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
redirect_to "/errors/error_500"
- 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"
Space inside parentheses detected. Open
if ( @notification.update(notification_params) )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Use a guard clause instead of wrapping the code inside a conditional expression. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
Use a guard clause instead of wrapping the code inside a conditional expression
Example:
# bad
def test
if something
work
end
end
# good
def test
return unless something
work
end
# also good
def test
work if something
end
# bad
if something
raise 'exception'
else
ok
end
# good
raise 'exception' if something
ok
Don't use parentheses around a method call. Open
if ( is_employee? )
- 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
flash[:success] = "A notificação foi criada com sucesso."
- 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"
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around a method call. Open
if ( is_employee? )
- 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
@assignee = Employee.exists?(@notification.employee_id) ? Employee.find_by_id(@notification.employee_id).name : "Desconhecido"
- 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"
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around the condition of an if
. Open
if (@notification.save)
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around the condition of an if
. Open
if ( logged_in? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Don't use parentheses around a method call. Open
if ( is_employee? )
- 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
redirect_to "/errors/error_500"
- 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"
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around the condition of an if
. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for the presence of superfluous parentheses around the condition of if/unless/while/until.
Example:
# bad
x += 1 while (x < 10)
foo unless (bar || baz)
if (x > 10)
elsif (x < 3)
end
# good
x += 1 while x < 10
foo unless bar || baz
if x > 10
elsif x < 3
end
Space inside parentheses detected. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Space inside parentheses detected. Open
params.require(:notification).permit( :title,
- Read upRead up
- Exclude checks
Checks for spaces inside ordinary round parentheses.
Example:
# bad
f( 3)
g = (a + 3 )
# good
f(3)
g = (a + 3)
Don't use parentheses around a method call. Open
if ( is_employee? )
- Read upRead up
- Exclude checks
This cop checks for redundant parentheses.
Example:
# bad
(x) if ((y.z).nil?)
# good
x if y.z.nil?
Don't use parentheses around a method call. Open
if ( @notification.update(notification_params) )
- 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
redirect_to "/errors/error_500"
- 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
redirect_to "/errors/error_500"
- 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"