Assignment Branch Condition size for post_grades is too high. [20.49/15] Open
def post_grades
if is_secretary?
@classroom = Classroom.find(params[:id])
@subject = Subject.find(params[:subject_id])
@alumn = Alumn.find(params[:alumn_id])
- 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. [12/10] Open
def post_grades
if is_secretary?
@classroom = Classroom.find(params[:id])
@subject = Subject.find(params[:subject_id])
@alumn = Alumn.find(params[:alumn_id])
- 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.
GradesController#post_grades has approx 7 statements Open
def post_grades
- 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.)
GradesController has at least 5 instance variables Open
class GradesController < ApplicationController
- Read upRead up
- Exclude checks
Too Many Instance Variables
is a special case of LargeClass
.
Example
Given this configuration
TooManyInstanceVariables:
max_instance_variables: 3
and this code:
class TooManyInstanceVariables
def initialize
@arg_1 = :dummy
@arg_2 = :dummy
@arg_3 = :dummy
@arg_4 = :dummy
end
end
Reek would emit the following warning:
test.rb -- 5 warnings:
[1]:TooManyInstanceVariables has at least 4 instance variables (TooManyInstanceVariables)
GradesController assumes too much for instance variable '@classroom' Open
class GradesController < 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.
GradesController#self.create calls 'subject.id' 2 times Open
if !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
@grade = Grade.new(alumn_id: alumn.id, subject_id: subject.id,
- 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.
GradesController assumes too much for instance variable '@subject' Open
class GradesController < 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.
GradesController assumes too much for instance variable '@grade' Open
class GradesController < 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.
GradesController#self.create calls 'alumn.id' 2 times Open
if !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
@grade = Grade.new(alumn_id: alumn.id, subject_id: subject.id,
- 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.
GradesController assumes too much for instance variable '@alumn' Open
class GradesController < 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.
GradesController assumes too much for instance variable '@current_user' Open
class GradesController < 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.
Use a guard clause instead of wrapping the code inside a conditional expression. Open
if (is_secretary? or is_principal?)
- 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
Space missing after colon. Open
@grades = Grade.where(classroom_id:@classroom.id).where(subject_id:@subject.id)
- Read upRead up
- Exclude checks
Checks for colon (:) not followed by some kind of space. N.B. this cop does not handle spaces after a ternary operator, which are instead handled by Layout/SpaceAroundOperators.
Example:
# bad
def f(a:, b:2); {a:3}; end
# good
def f(a:, b: 2); {a: 3}; end
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
Line is too long. [108/80] Open
@grade = Grade.find_by_classroom_id_and_subject_id_and_alumn_id(@classroom.id, @subject.id, @alumn.id)
- Exclude checks
Use a guard clause instead of wrapping the code inside a conditional expression. Open
if is_secretary?
- 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 !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
- 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 missing after colon. Open
if !Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?
- Read upRead up
- Exclude checks
Checks for colon (:) not followed by some kind of space. N.B. this cop does not handle spaces after a ternary operator, which are instead handled by Layout/SpaceAroundOperators.
Example:
# bad
def f(a:, b:2); {a:3}; end
# good
def f(a:, b: 2); {a: 3}; end
Space missing after comma. Open
GradeHistoriesController.create(@grade,@current_user)
- Read upRead up
- Exclude checks
Checks for comma (,) not followed by some kind of space.
Example:
# bad
[1,2]
{ foo:bar,}
# good
[1, 2]
{ foo:bar, }
Do not put a space between a method name and the opening parenthesis. Open
def self.update_alumn (alumn)
- Read upRead up
- Exclude checks
Checks for space between a method name and a left parenthesis in defs.
Example:
# bad
def func (x) end
def method= (y) end
# good
def func(x) end
def method=(y) end
Use ||
instead of or
. Open
if (is_secretary? or is_principal?)
- Read upRead up
- Exclude checks
This cop checks for uses of and
and or
, and suggests using &&
and
|| instead
. It can be configured to check only in conditions, or in
all contexts.
Example: EnforcedStyle: always (default)
# bad
foo.save and return
# bad
if foo and bar
end
# good
foo.save && return
# good
if foo && bar
end
Example: EnforcedStyle: conditionals
# bad
if foo and bar
end
# good
foo.save && return
# good
foo.save and return
# good
if foo && bar
end
Don't use parentheses around the condition of an if
. Open
if (is_secretary? or is_principal?)
- 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
Favor unless
over if
for negative conditions. Open
if !Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?
GradesController.create(alumn)
end
- Read upRead up
- Exclude checks
Checks for uses of if with a negated condition. Only ifs without else are considered. There are three different styles:
- both
- prefix
- postfix
Example: EnforcedStyle: both (default)
# enforces `unless` for `prefix` and `postfix` conditionals
# bad
if !foo
bar
end
# good
unless foo
bar
end
# bad
bar if !foo
# good
bar unless foo
Example: EnforcedStyle: prefix
# enforces `unless` for just `prefix` conditionals
# bad
if !foo
bar
end
# good
unless foo
bar
end
# good
bar if !foo
Example: EnforcedStyle: postfix
# enforces `unless` for just `postfix` conditionals
# bad
bar if !foo
# good
bar unless foo
# good
if !foo
bar
end
Space missing after colon. Open
@grades = Grade.where(classroom_id:@classroom.id).where(subject_id:@subject.id)
- Read upRead up
- Exclude checks
Checks for colon (:) not followed by some kind of space. N.B. this cop does not handle spaces after a ternary operator, which are instead handled by Layout/SpaceAroundOperators.
Example:
# bad
def f(a:, b:2); {a:3}; end
# good
def f(a:, b: 2); {a: 3}; end
Do not put a space between a method name and the opening parenthesis. Open
def self.create (alumn)
- Read upRead up
- Exclude checks
Checks for space between a method name and a left parenthesis in defs.
Example:
# bad
def func (x) end
def method= (y) end
# good
def func(x) end
def method=(y) end
Line is too long. [85/80] Open
@grades = Grade.where(classroom_id:@classroom.id).where(subject_id:@subject.id)
- Exclude checks
Align the elements of a hash literal if they span more than one line. Open
classroom_id: alumn.classroom_id)
- Read upRead up
- Exclude checks
Check that the keys, separators, and values of a multi-line hash literal are aligned according to configuration. The configuration options are:
- key (left align keys)
- separator (align hash rockets and colons, right align keys)
- table (left align keys, hash rockets, and values)
The treatment of hashes passed as the last argument to a method call can also be configured. The options are:
- always_inspect
- always_ignore
- ignore_implicit (without curly braces)
- ignore_explicit (with curly braces)
Example:
# EnforcedHashRocketStyle: key (default)
# EnforcedColonStyle: key (default)
# good
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
# bad
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
Example:
# EnforcedHashRocketStyle: separator
# EnforcedColonStyle: separator
#good
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
#bad
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
{
:foo => bar,
:ba => baz
}
Example:
# EnforcedHashRocketStyle: table
# EnforcedColonStyle: table
#good
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
#bad
{
foo: bar,
ba: baz
}
{
:foo => bar,
:ba => baz
}
Space missing after colon. Open
if !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
- Read upRead up
- Exclude checks
Checks for colon (:) not followed by some kind of space. N.B. this cop does not handle spaces after a ternary operator, which are instead handled by Layout/SpaceAroundOperators.
Example:
# bad
def f(a:, b:2); {a:3}; end
# good
def f(a:, b: 2); {a: 3}; end
Favor unless
over if
for negative conditions. Open
if !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
@grade = Grade.new(alumn_id: alumn.id, subject_id: subject.id,
classroom_id: alumn.classroom_id)
@grade.save
end
- Read upRead up
- Exclude checks
Checks for uses of if with a negated condition. Only ifs without else are considered. There are three different styles:
- both
- prefix
- postfix
Example: EnforcedStyle: both (default)
# enforces `unless` for `prefix` and `postfix` conditionals
# bad
if !foo
bar
end
# good
unless foo
bar
end
# bad
bar if !foo
# good
bar unless foo
Example: EnforcedStyle: prefix
# enforces `unless` for just `prefix` conditionals
# bad
if !foo
bar
end
# good
unless foo
bar
end
# good
bar if !foo
Example: EnforcedStyle: postfix
# enforces `unless` for just `postfix` conditionals
# bad
bar if !foo
# good
bar unless foo
# good
if !foo
bar
end
Prefer single-quoted strings when you don't need string interpolation or special symbols. Open
render "grades/index"
- 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"
Indent access modifiers like private
. Open
private
- Read upRead up
- Exclude checks
Modifiers should be indented as deep as method definitions, or as deep as the class/module keyword, depending on configuration.
Example: EnforcedStyle: indent (default)
# bad
class Plumbus
private
def smooth; end
end
# good
class Plumbus
private
def smooth; end
end
Example: EnforcedStyle: outdent
# bad
class Plumbus
private
def smooth; end
end
# good
class Plumbus
private
def smooth; end
end
Use next
to skip iteration. Open
if !(Grade.where(alumn_id:alumn.id).where(subject_id: subject.id).exists?)
- Read upRead up
- Exclude checks
Use next
to skip iteration instead of a condition at the end.
Example: EnforcedStyle: skipmodifierifs (default)
# bad
[1, 2].each do |a|
if a == 1
puts a
end
end
# good
[1, 2].each do |a|
next unless a == 1
puts a
end
# good
[1, 2].each do |o|
puts o unless o == 1
end
Example: EnforcedStyle: always
# With `always` all conditions at the end of an iteration needs to be
# replaced by next - with `skip_modifier_ifs` the modifier if like
# this one are ignored: `[1, 2].each { |a| return 'yes' if a == 1 }`
# bad
[1, 2].each do |o|
puts o unless o == 1
end
# bad
[1, 2].each do |a|
if a == 1
puts a
end
end
# good
[1, 2].each do |a|
next unless a == 1
puts a
end