sanger/sequencescape

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

Summary

Maintainability
B
6 hrs
Test Coverage
A
98%

Complex method Submission::LinearRequestGraph#create_request_chain! (101.7)
Open

  def create_request_chain!(request_type_and_multiplier_pairs, source_data_set, multiplexing_assets, &block) # rubocop:todo Metrics/CyclomaticComplexity
    raise StandardError, 'No request types specified!' if request_type_and_multiplier_pairs.empty?

    request_type, multiplier = request_type_and_multiplier_pairs.shift

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 create_request_chain! has a Cognitive Complexity of 32 (exceeds 5 allowed). Consider refactoring.
Open

  def create_request_chain!(request_type_and_multiplier_pairs, source_data_set, multiplexing_assets, &block) # rubocop:todo Metrics/CyclomaticComplexity
    raise StandardError, 'No request types specified!' if request_type_and_multiplier_pairs.empty?

    request_type, multiplier = request_type_and_multiplier_pairs.shift

Severity: Minor
Found in app/models/submission/linear_request_graph.rb - About 4 hrs 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 create_request_chain! has 43 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def create_request_chain!(request_type_and_multiplier_pairs, source_data_set, multiplexing_assets, &block) # rubocop:todo Metrics/CyclomaticComplexity
    raise StandardError, 'No request types specified!' if request_type_and_multiplier_pairs.empty?

    request_type, multiplier = request_type_and_multiplier_pairs.shift

Severity: Minor
Found in app/models/submission/linear_request_graph.rb - About 1 hr to fix

    Complex method Submission::LinearRequestGraph#associate_built_requests (29.7)
    Open

      def associate_built_requests(assets) # rubocop:todo Metrics/AbcSize
        assets
          .map(&:requests)
          .flatten
          .each do |request|

    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::LinearRequestGraph#build_request_type_multiplier_pairs has approx 7 statements
    Open

      def build_request_type_multiplier_pairs # 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::LinearRequestGraph#build_request_graph! has approx 6 statements
    Open

      def build_request_graph!(multiplexing_assets = nil)

    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::LinearRequestGraph#associate_built_requests contains iterators nested 2 deep
    Open

              comments.split("\n").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::LinearRequestGraph#create_request_chain! contains iterators nested 2 deep
    Open

                comments.split("\n").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::LinearRequestGraph#create_request_chain! has approx 28 statements
    Open

      def create_request_chain!(request_type_and_multiplier_pairs, source_data_set, multiplexing_assets, &block) # rubocop:todo Metrics/CyclomaticComplexity

    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::LinearRequestGraph#create_target_asset_for! is controlled by argument 'source_asset'
    Open

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

    Control Parameter is a special case of Control Couple

    Example

    A simple example would be the "quoted" parameter in the following method:

    def write(quoted)
      if quoted
        write_quoted @value
      else
        write_unquoted @value
      end
    end

    Fixing those problems is out of the scope of this document but an easy solution could be to remove the "write" method alltogether and to move the calls to "writequoted" / "writeunquoted" in the initial caller of "write".

    Submission::LinearRequestGraph#create_request_chain! calls 'request_type.for_multiplexing?' 3 times
    Open

            if request_type.for_multiplexing?
              multiplexing_assets || Array.new(source_data_set.length, create_target_asset_for!(request_type))
            else
              source_data_set.map { |source_data| create_target_asset_for!(request_type, source_data.asset) }
            end

    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.

    Submission::LinearRequestGraph#create_request_chain! calls 'source_data_set[index]' 2 times
    Open

                source_asset = request_type.no_target_asset? ? source_data_set[index].first : asset
                SourceData.new(source_asset, source_data_set[index].qc_metric, nil)

    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.

    Submission::LinearRequestGraph#create_request_chain! calls 'request_type_and_multiplier_pairs.empty?' 2 times
    Open

        raise StandardError, 'No request types specified!' if request_type_and_multiplier_pairs.empty?
    
        request_type, multiplier = request_type_and_multiplier_pairs.shift
    
        # rubocop:todo Metrics/BlockLength

    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.

    Submission::LinearRequestGraph#create_request_chain! calls 'target_assets.uniq' 2 times
    Open

                target_assets.uniq.map { |asset| SourceData.new(asset, criteria, nil) }
              else
                associate_built_requests(target_assets.uniq.compact)

    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.

    Submission::LinearRequestGraph#create_request_chain! calls 'source_data.asset' 2 times
    Open

              source_data_set.map { |source_data| create_target_asset_for!(request_type, source_data.asset) }
            end
          yield(target_assets) if block && request_type.for_multiplexing?
    
          # Now we can iterate over the source assets and target assets building the requests between them.

    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.

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

      def associate_built_requests(assets) # rubocop:todo Metrics/AbcSize
        assets
          .map(&:requests)
          .flatten
          .each do |request|
    Severity: Minor
    Found in app/models/submission/linear_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::LinearRequestGraph#create_target_asset_for! doesn't depend on instance state (maybe move it to another class?)
    Open

      def create_target_asset_for!(request_type, source_asset = nil)

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

    Submission::LinearRequestGraph#create_request_chain! performs a nil-check
    Open

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

    Submission::LinearRequestGraph#build_request_graph! has the variable name 'a'
    Open

          ) { |a| mx_assets_tmp = a }

    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::LinearRequestGraph#build_request_type_multiplier_pairs has the variable name 'k'
    Open

            .new { |h, k| h[k] = 1 }
            .tap do |multipliers|
              requested_multipliers = 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.

    Submission::LinearRequestGraph#build_request_type_multiplier_pairs 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::LinearRequestGraph#build_request_type_multiplier_pairs 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.

    TODO found
    Open

              # TODO: AssetLink is supposed to disappear at some point in the future because it makes no real sense

    There are no issues that match your filters.

    Category
    Status