sanger/sequencescape

View on GitHub
app/models/submission/flexible_request_graph.rb

Summary

Maintainability
A
1 hr
Test Coverage
A
94%

Complex method Submission::FlexibleRequestGraph::ChainLink#build! (40.0)
Open

    def build! # rubocop:todo Metrics/AbcSize
      multiplier.times do |_|
        # Now we can iterate over the source assets and target assets building the requests between them.
        # Ensure that the request has the correct comments on it, and that the aliquots of the source asset
        # are transferred into the destination if the request does not do this in some manner itself.

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

Method build! has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring.
Open

    def build! # rubocop:todo Metrics/AbcSize
      multiplier.times do |_|
        # Now we can iterate over the source assets and target assets building the requests between them.
        # Ensure that the request has the correct comments on it, and that the aliquots of the source asset
        # are transferred into the destination if the request does not do this in some manner itself.
Severity: Minor
Found in app/models/submission/flexible_request_graph.rb - About 55 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Submission::FlexibleRequestGraph::OrderMethods#build_request_graph! refers to 'chain' more than self (maybe move it to another class?)
Open

        chain.build!
        chain.multiplexing_assets if chain.multiplexed?

Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.

Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.

Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.

Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.

Example

Running Reek on:

class Warehouse
  def sale_price(item)
    (item.price - item.rebate) * @vat
  end
end

would report:

Warehouse#total_price refers to item more than self (FeatureEnvy)

since this:

(item.price - item.rebate)

belongs to the Item class, not the Warehouse.

Submission::FlexibleRequestGraph::ChainLink#self.build! has 4 parameters
Open

    def self.build!(request_type, multiplier, source_assets_qc_metrics, chain)

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.

Submission::FlexibleRequestGraph::RequestChain has at least 6 instance variables
Open

  class RequestChain

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)

Submission::FlexibleRequestGraph::ChainLink#build! has approx 9 statements
Open

    def build! # rubocop:todo Metrics/AbcSize

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.)

Submission::FlexibleRequestGraph::ChainLink#create_target_asset refers to 'asset' more than self (maybe move it to another class?)
Open

        asset.generate_barcode
        asset.generate_name(source_asset.try(:name) || asset.try(:human_barcode)&.to_s)

Feature Envy occurs when a code fragment references another object more often than it references itself, or when several clients do the same series of manipulations on a particular type of object.

Feature Envy reduces the code's ability to communicate intent: code that "belongs" on one class but which is located in another can be hard to find, and may upset the "System of Names" in the host class.

Feature Envy also affects the design's flexibility: A code fragment that is in the wrong class creates couplings that may not be natural within the application's domain, and creates a loss of cohesion in the unwilling host class.

Feature Envy often arises because it must manipulate other objects (usually its arguments) to get them into a useful form, and one force preventing them (the arguments) doing this themselves is that the common knowledge lives outside the arguments, or the arguments are of too basic a type to justify extending that type. Therefore there must be something which 'knows' about the contents or purposes of the arguments. That thing would have to be more than just a basic type, because the basic types are either containers which don't know about their contents, or they are single objects which can't capture their relationship with their fellows of the same type. So, this thing with the extra knowledge should be reified into a class, and the utility method will most likely belong there.

Example

Running Reek on:

class Warehouse
  def sale_price(item)
    (item.price - item.rebate) * @vat
  end
end

would report:

Warehouse#total_price refers to item more than self (FeatureEnvy)

since this:

(item.price - item.rebate)

belongs to the Item class, not the Warehouse.

Submission::FlexibleRequestGraph::MultiplexedLink#associate_built_requests! contains iterators nested 2 deep
Open

        comments.each { |comment| request.comments.create!(user: user, description: comment) } if comments.present?

A Nested Iterator occurs when a block contains another block.

Example

Given

class Duck
  class << self
    def duck_names
      %i!tick trick track!.each do |surname|
        %i!duck!.each do |last_name|
          puts "full name is #{surname} #{last_name}"
        end
      end
    end
  end
end

Reek would report the following warning:

test.rb -- 1 warning:
  [5]:Duck#duck_names contains iterators nested 2 deep (NestedIterators)

Submission::FlexibleRequestGraph::ChainLink#build! contains iterators nested 3 deep
Open

                comments.each { |comment| request.comments.create!(user: user, description: comment) }

A Nested Iterator occurs when a block contains another block.

Example

Given

class Duck
  class << self
    def duck_names
      %i!tick trick track!.each do |surname|
        %i!duck!.each do |last_name|
          puts "full name is #{surname} #{last_name}"
        end
      end
    end
  end
end

Reek would report the following warning:

test.rb -- 1 warning:
  [5]:Duck#duck_names contains iterators nested 2 deep (NestedIterators)

Submission::FlexibleRequestGraph::RequestChain#build! has approx 7 statements
Open

    def build!

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.)

Complex method Submission::FlexibleRequestGraph::MultiplexedLink#associate_built_requests! (23.6)
Open

    def associate_built_requests!
      downstream_requests.each do |request|
        request.update!(initial_study: nil) if request.initial_study != study
        request.update!(initial_project: nil) if request.initial_project != project
        comments.each { |comment| request.comments.create!(user: user, description: comment) } if comments.present?

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

Submission::FlexibleRequestGraph::MultiplexedLink has no descriptive comment
Open

  class MultiplexedLink

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

Submission::FlexibleRequestGraph::OrderMethods has no descriptive comment
Open

  module OrderMethods

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

Submission::FlexibleRequestGraph::ChainLink has initialize method
Open

  module ChainLink

A module is usually a mixin, so when an #initialize method is present it is hard to tell initialization order and parameters so having #initialize in a module is usually a bad idea.

Example

The Foo module below contains a method initialize. Although class B inherits from A, the inclusion of Foo stops A#initialize from being called.

class A
  def initialize(a)
    @a = a
  end
end

module Foo
  def initialize(foo)
    @foo = foo
  end
end

class B < A
  include Foo

  def initialize(b)
    super('bar')
    @b = b
  end
end

A simple solution is to rename Foo#initialize and call that method by name:

module Foo
  def setup_foo_module(foo)
    @foo = foo
  end
end

class B < A
  include Foo

  def initialize(b)
    super 'bar'
    setup_foo_module('foo')
    @b = b
  end
end

Submission::FlexibleRequestGraph::RequestChain has no descriptive comment
Open

  class RequestChain

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

Submission::FlexibleRequestGraph::RequestChainError has no descriptive comment
Open

  class RequestChainError < RuntimeError

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

Submission::FlexibleRequestGraph::UnplexedLink has no descriptive comment
Open

  class UnplexedLink

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

Method associate_built_requests! has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

    def associate_built_requests!
      downstream_requests.each do |request|
        request.update!(initial_study: nil) if request.initial_study != study
        request.update!(initial_project: nil) if request.initial_project != project
        comments.each { |comment| request.comments.create!(user: user, description: comment) } if comments.present?
Severity: Minor
Found in app/models/submission/flexible_request_graph.rb - About 25 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method build! has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

    def build!
      raise RequestChainError, 'Request chains can only be built once' if built?
      raise StandardError, 'No request types specified!' if request_types.empty?

      request_types.inject(source_assets_qc_metrics) do |source_assets_qc_metrics_memo, request_type|
Severity: Minor
Found in app/models/submission/flexible_request_graph.rb - About 25 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Submission::FlexibleRequestGraph::MultiplexedLink has missing safe method 'associate_built_requests!'
Open

    def associate_built_requests!

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.

Submission::FlexibleRequestGraph::RequestChain has missing safe method 'build!'
Open

    def build!

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.

Submission::FlexibleRequestGraph::RequestChain#multipliers has the variable name 'v'
Open

            requested_multipliers.each { |k, v| multipliers[k.to_s] = v.to_i }

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.

Submission::FlexibleRequestGraph::RequestChain#multipliers has the variable name 'h'
Open

          .new { |h, k| h[k] = 1 }

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.

Submission::FlexibleRequestGraph::RequestChain#multipliers has the variable name 'k'
Open

          .new { |h, k| h[k] = 1 }
          .tap do |multipliers|
            requested_multipliers = order.request_options.try(:[], :multiplier) || {}
            requested_multipliers.each { |k, v| multipliers[k.to_s] = v.to_i }

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.

There are no issues that match your filters.

Category
Status