RnD-Soft/lusnoc

View on GitHub

Showing 74 of 74 total issues

Lusnoc::Session assumes too much for instance variable '@session_die_cb'
Open

  class Session
Severity: Minor
Found in lib/lusnoc/session.rb by reek

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.

Lusnoc::Mutex#acquisition_loop! calls 'prepare_guard(session, key).run do ... end' 2 times
Open

          prepare_guard(session, key).run do
            return yield(self)
          end
        end

Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Mutex#acquisition_loop! calls 'prepare_guard(session, key)' 2 times
Open

          prepare_guard(session, key).run do
            return yield(self)
          end
        end

Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Mutex#acquisition_loop! calls 'session.id' 2 times
Open

        logger.debug("Mutex[#{key}] run acquisition loop for Session[#{session.name}:#{session.id}]")
        t.loop! do
          session.alive!(TimeoutError)
          wait_for_key_released(key, t.left)

Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Session assumes too much for instance variable '@id'
Open

  class Session
Severity: Minor
Found in lib/lusnoc/session.rb by reek

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.

Lusnoc::TimeoutError has no descriptive comment
Open

  class TimeoutError < Error; end
Severity: Minor
Found in lib/lusnoc/exceptions.rb by reek

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

Lusnoc::Mutex assumes too much for instance variable '@on_mutex_lost'
Open

  class Mutex
Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Mutex#acquisition_loop! calls 'acquire(key, session, value)' 2 times
Open

        if acquire(key, session, value)
          prepare_guard(session, key).run do
            return yield(self)
          end
        end
Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Mutex has no descriptive comment
Open

  class Mutex
Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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

Lusnoc::Session has no descriptive comment
Open

  class Session
Severity: Minor
Found in lib/lusnoc/session.rb by reek

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

Lusnoc::Watcher#build_wait_condition calls 'max_consul_wait.to_i' 2 times
Open

        max = [time_left.to_i, max_consul_wait.to_i].max
        "&wait=#{max}s"
      elsif max_consul_wait
        "&wait=#{max_consul_wait.to_i}s"
Severity: Minor
Found in lib/lusnoc/watcher.rb by reek

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.

Complex method Lusnoc::Mutex#prepare_guard (22.6)
Open

      def prepare_guard(session, key)
        Lusnoc::Guard.new(build_url("/v1/kv/#{key}")) do |guard|
          guard.condition do |body|
            JSON.parse(body).first['Session'] == session.id rescue false
          end
Severity: Minor
Found in lib/lusnoc/mutex.rb by flog

Flog calculates the ABC score for methods. The ABC score is based on assignments, branches (method calls), and conditions.

You can read more about ABC metrics or the flog tool

Lusnoc::Configuration#logger is a writable attribute
Open

    attr_accessor :url, :acl_token, :logger, :http_timeout
Severity: Minor
Found in lib/lusnoc/configuration.rb by reek

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)

Lusnoc#http_put performs a nil-check
Open

      data = value.is_a?(String) ? value : JSON.generate(value) unless value.nil?
Severity: Minor
Found in lib/lusnoc.rb by reek

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)

Lusnoc::Configuration#url is a writable attribute
Open

    attr_accessor :url, :acl_token, :logger, :http_timeout
Severity: Minor
Found in lib/lusnoc/configuration.rb by reek

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)

Lusnoc::Session has missing safe method 'alive!'
Open

    def alive!(exception_class = ExpiredError)
Severity: Minor
Found in lib/lusnoc/session.rb by reek

A candidate method for the Missing Safe Method smell are methods whose names end with an exclamation mark.

An exclamation mark in method names means (the explanation below is taken from here ):

The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.

Such a method is called Missing Safe Method if and only if her non-bang version does not exist and this method is reported as a smell.

Example

Given

class C
  def foo; end
  def foo!; end
  def bar!; end
end

Reek would report bar! as Missing Safe Method smell but not foo!.

Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:

class Parent
  def foo; end
end

module Dangerous
  def foo!; end
end

class Son < Parent
  include Dangerous
end

class Daughter < Parent
end

In this example, Reek would not report the Missing Safe Method smell for the method foo of the Dangerous module.

Lusnoc#with_retry performs a nil-check
Open

            raise if klass.nil?
Severity: Minor
Found in lib/lusnoc.rb by reek

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)

Lusnoc::Mutex takes parameters ['key', 'session'] to 4 methods
Open

      def acquire(key, session, value)
        resp = Lusnoc.http_put(build_url("/v1/kv/#{key}?acquire=#{session.id}"), value, timeout: 1)
        return false if resp.body.chomp != 'true'

        @owner = Thread.current
Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

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.

Lusnoc::Mutex has missing safe method 'acquisition_loop!'
Open

      def acquisition_loop!(key, session, value, t)
Severity: Minor
Found in lib/lusnoc/mutex.rb by reek

A candidate method for the Missing Safe Method smell are methods whose names end with an exclamation mark.

An exclamation mark in method names means (the explanation below is taken from here ):

The ! in method names that end with ! means, “This method is dangerous”—or, more precisely, this method is the “dangerous” version of an otherwise equivalent method, with the same name minus the !. “Danger” is relative; the ! doesn’t mean anything at all unless the method name it’s in corresponds to a similar but bang-less method name. So, for example, gsub! is the dangerous version of gsub. exit! is the dangerous version of exit. flatten! is the dangerous version of flatten. And so forth.

Such a method is called Missing Safe Method if and only if her non-bang version does not exist and this method is reported as a smell.

Example

Given

class C
  def foo; end
  def foo!; end
  def bar!; end
end

Reek would report bar! as Missing Safe Method smell but not foo!.

Reek reports this smell only in a class context, not in a module context in order to allow perfectly legit code like this:

class Parent
  def foo; end
end

module Dangerous
  def foo!; end
end

class Son < Parent
  include Dangerous
end

class Daughter < Parent
end

In this example, Reek would not report the Missing Safe Method smell for the method foo of the Dangerous module.

Lusnoc::Configuration#http_timeout is a writable attribute
Open

    attr_accessor :url, :acl_token, :logger, :http_timeout
Severity: Minor
Found in lib/lusnoc/configuration.rb by reek

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)
Severity
Category
Status
Source
Language