Stats::Views::UsageHack::StorageCache assumes too much for instance variable '@cache' Open
class StorageCache < Stats::Storage
- 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.
Stats::Views::UsageHack::StorageFake assumes too much for instance variable '@cached_keys' Open
class StorageFake < Stats::Storage
- 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.
Stats::Views::UsageHack::StorageFake#mget calls 'key.to_s' 2 times Open
@cached_keys << key.to_s unless @cached_keys.include?(key.to_s)
- 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.
Stats::Views::UsageHack#storage performs a nil-check Open
raise NoStorageException if @storage.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)
Stats::Views::UsageHack::StorageCache#cache is a writable attribute Open
attr_accessor :cache
- Read upRead up
- Exclude checks
A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.
The same holds to a lesser extent for getters, but Reek doesn't flag those.
Example
Given:
class Klass
attr_accessor :dummy
end
Reek would emit the following warning:
reek test.rb
test.rb -- 1 warning:
[2]:Klass declares the writable attribute dummy (Attribute)