coreSegmentFault/view_delegates

View on GitHub

Showing 78 of 78 total issues

ViewDelegates::ViewDelegate#cache calls 'self.class.delegate_cache' 2 times
Open

            result = self.class.delegate_cache.get value_key
            if result.nil?
              result = render_method.bind(self).call(view, local_params: local_params)
              self.class.delegate_cache.add key: value_key, value: result

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.

ViewDelegates::ViewDelegate#render calls 'self.class' 7 times
Open

      self.class.view_locals&.each do |method|
        locals[method] = send(method)
      end
      self.ar_models = {}
      self.view_helpers = {}

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.

ViewDelegates::ViewDelegate assumes too much for instance variable '@polymorph_function'
Open

  class ViewDelegate

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.

ViewDelegates::ViewDelegate#initialize calls 'self.class' 2 times
Open

      self.class.ar_models&.each do |t|
        send("#{t}=", view_data[t]) if view_data[t]
      end
      self.class.properties&.each do |t|

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.

ViewDelegates::ViewDelegate#initialize calls 'send("#{t}=", view_data[t])' 2 times
Open

        send("#{t}=", view_data[t]) if view_data[t]
      end
      self.class.properties&.each do |t|
        send("#{t}=", view_data[t]) if view_data[t]

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.

ViewDelegates::ViewDelegate#cache calls 'self.class' 2 times
Open

            result = self.class.delegate_cache.get value_key
            if result.nil?
              result = render_method.bind(self).call(view, local_params: local_params)
              self.class.delegate_cache.add key: value_key, value: result

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 ViewDelegates::ViewDelegate::model (22.4)
Open

      def model(method, properties: [])
        attr_accessor method
        # Add the method name to the array of delegate models
        self.ar_models += [method]
        # Define a setter for the model

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

ViewDelegates::Cache#entries is a writable attribute
Open

    attr_accessor :entries
Severity: Minor
Found in lib/view_delegates/poros/cache.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)

ViewDelegates::ViewDelegate#model_to_struct doesn't depend on instance state (maybe move it to another class?)
Open

    def model_to_struct(model, struct)

A Utility Function is any instance method that has no dependency on the state of the instance.

ViewDelegates::ViewDelegate#delegate_cache is a writable attribute
Open

      attr_accessor :delegate_cache

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)

ViewDelegates::ViewDelegate#polymorph_function is a writable attribute
Open

      attr_accessor :polymorph_function

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)

ViewDelegates::ViewDelegate#cache performs a nil-check
Open

            if result.nil?

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)

Complex method ViewDelegates::ViewDelegate#initialize (20.3)
Open

    def initialize(view_data = {})
      self.class.ar_models&.each do |t|
        send("#{t}=", view_data[t]) if view_data[t]
      end
      self.class.properties&.each do |t|

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

ViewDelegates::Cache#get has the variable name 'e'
Open

      index = @entries.index { |e| e.key == key } rescue byebug
Severity: Minor
Found in lib/view_delegates/poros/cache.rb by reek

An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

ViewDelegates::ViewDelegate#initialize has the variable name 't'
Open

      self.class.ar_models&.each do |t|
        send("#{t}=", view_data[t]) if view_data[t]
      end
      self.class.properties&.each do |t|

An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

ViewDelegates::ViewDelegate#model_to_struct has the variable name 'k'
Open

      struct_members.each do |k|

An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

ViewDelegates::ViewDelegate#model_array has the variable name 'e'
Open

          model_array = model_array.map {|e| model_to_struct(e, model_delegate)}

An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

Redundant self detected.
Open

    self.ar_models = self.ar_models || []

This cop checks for redundant uses of self.

The usage of self is only needed when:

  • Sending a message to same object with zero arguments in presence of a method name clash with an argument or a local variable.

  • Calling an attribute writer to prevent an local variable assignment.

Note, with using explicit self you can only send messages with public or protected scope, you cannot send private messages this way.

Note we allow uses of self with operators because it would be awkward otherwise.

Example:

# bad
def foo(bar)
  self.baz
end

# good
def foo(bar)
  self.bar  # Resolves name clash with the argument.
end

def foo
  bar = 1
  self.bar  # Resolves name clash with the local variable.
end

def foo
  %w[x y z].select do |bar|
    self.bar == bar  # Resolves name clash with argument of the block.
  end
end

Prefer to_s over string interpolation.
Open

      "#{to_s.gsub(/^.*::/, '')}".sub(/Delegate/, ''.freeze).concat('Helper').underscore

This cop checks for strings that are just an interpolated expression.

Example:

# bad
"#{@var}"

# good
@var.to_s

# good if @var is already a String
@var

Remove debugger entry point byebug.
Open

      index = @entries.index { |e| e.key == key } rescue byebug
Severity: Minor
Found in lib/view_delegates/poros/cache.rb by rubocop

This cop checks for calls to debugger or pry.

Example:

# bad (ok during development)

# using pry
def some_method
  binding.pry
  do_something
end

Example:

# bad (ok during development)

# using byebug
def some_method
  byebug
  do_something
end

Example:

# good

def some_method
  do_something
end
Severity
Category
Status
Source
Language