Mass assignment is not restricted using attr_accessible Open
class Finance::PrepaidBillingStrategy < Finance::BillingStrategy
- Read upRead up
- Exclude checks
This warning comes up if a model does not limit what attributes can be set through mass assignment.
In particular, this check looks for attr_accessible
inside model definitions. If it is not found, this warning will be issued.
Brakeman also warns on use of attr_protected
- especially since it was found to be vulnerable to bypass. Warnings for mass assignment on models using attr_protected
will be reported, but at a lower confidence level.
Note that disabling mass assignment globally will suppress these warnings.
Finance::PrepaidBillingStrategy#bill_plan_change_for_unbilled_contract has approx 11 statements Open
def bill_plan_change_for_unbilled_contract(contract, period)
- 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.)
Finance::PrepaidBillingStrategy#upgrade_on_plan_change has 4 parameters Open
def upgrade_on_plan_change(contract, old_plan, plan, period)
- Read upRead up
- Exclude checks
A Long Parameter List
occurs when a method has a lot of parameters.
Example
Given
class Dummy
def long_list(foo,bar,baz,fling,flung)
puts foo,bar,baz,fling,flung
end
end
Reek would report the following warning:
test.rb -- 1 warning:
[2]:Dummy#long_list has 5 parameters (LongParameterList)
A common solution to this problem would be the introduction of parameter objects.
Finance::PrepaidBillingStrategy#daily has approx 12 statements Open
def daily(options = {})
- 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.)
Finance::PrepaidBillingStrategy#daily calls 'log_prefix(buyer)' 2 times Open
Rails.logger.info("#{log_prefix(buyer)} started daily billing and charging at #{now} (prepaid)")
bill_expired_trials(buyer, now)
only_on_days(now, 1) do
bill_fixed_costs(buyer, now)
- 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.
Finance::PrepaidBillingStrategy takes parameters ['contract', 'period'] to 4 methods Open
def bill_plan_change(contract, period)
if contract.never_billed?
bill_plan_change_for_unbilled_contract(contract, period)
else
bill_plan_change_for_billed_contract(contract, period)
- Read upRead up
- Exclude checks
In general, a Data Clump
occurs when the same two or three items frequently appear together in classes and parameter lists, or when a group of instance variable names start or end with similar substrings.
The recurrence of the items often means there is duplicate code spread around to handle them. There may be an abstraction missing from the code, making the system harder to understand.
Example
Given
class Dummy
def x(y1,y2); end
def y(y1,y2); end
def z(y1,y2); end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[2, 3, 4]:Dummy takes parameters [y1, y2] to 3 methods (DataClump)
A possible way to fix this problem (quoting from Martin Fowler):
The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects.
Finance::PrepaidBillingStrategy#invoices_to_finalize_of has unused parameter 'now' Open
def invoices_to_finalize_of(buyer, now)
- 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)