sanger/sequencescape

View on GitHub
app/models/plate.rb

Summary

Maintainability
C
1 day
Test Coverage
A
93%

Class Plate has 51 methods (exceeds 20 allowed). Consider refactoring.
Open

class Plate < Labware # rubocop:todo Metrics/ClassLength
  include Api::PlateIO::Extensions
  include ModelExtensions::Plate
  include Transfer::Associations
  include Transfer::State::PlateState
Severity: Major
Found in app/models/plate.rb - About 7 hrs to fix

    File plate.rb has 320 lines of code (exceeds 250 allowed). Consider refactoring.
    Open

    require 'lab_where_client'
    
    # https://github.com/sanger/sequencescape/raw/master/docs/images/plate.jpg
    #
    # A plate is a piece of labware made up of a number of {Well wells}. This class represents the physical piece of
    Severity: Minor
    Found in app/models/plate.rb - About 3 hrs to fix

      Complex method Plate::has_many#construct! (40.6)
      Open

          def construct! # rubocop:todo Metrics/AbcSize
            transaction do
              plate = proxy_association.owner
              plate
                .maps
      Severity: Minor
      Found in app/models/plate.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

      Method stock_wells has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.
      Open

        def stock_wells # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
          # Optimisation: if the plate is a stock plate then it's wells are it's stock wells!]
          if stock_plate?
            wells.with_pool_id.index_with { |w| [w] }
          else
      Severity: Minor
      Found in app/models/plate.rb - About 1 hr 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

      Complex method Plate#stock_wells (28.0)
      Open

        def stock_wells # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
          # Optimisation: if the plate is a stock plate then it's wells are it's stock wells!]
          if stock_plate?
            wells.with_pool_id.index_with { |w| [w] }
          else
      Severity: Minor
      Found in app/models/plate.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

      Plate#construct! refers to 'ids' more than self (maybe move it to another class?)
      Open

                  WellAttribute.import(ids.map { |well| { well_id: well } })
                  Uuid.import(
                    ids.map { |well| { resource_id: well, resource_type: well_type, external_id: Uuid.generate_uuid } }
                  )
      
      
      Severity: Minor
      Found in app/models/plate.rb by reek

      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.

      Plate#stock_wells contains iterators nested 2 deep
      Open

                storted_stock_wells = w.stock_wells.sort_by { |sw| sw.map.column_order }
      Severity: Minor
      Found in app/models/plate.rb by reek

      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)

      Plate#update_qc_values_with_parser contains iterators nested 2 deep
      Open

              well_updates.each do |attribute, value|
      Severity: Minor
      Found in app/models/plate.rb by reek

      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)

      Plate#construct! has approx 13 statements
      Open

          def construct! # rubocop:todo Metrics/AbcSize
      Severity: Minor
      Found in app/models/plate.rb by reek

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

      Plate#update_qc_values_with_parser refers to 'parser' more than self (maybe move it to another class?)
      Open

            parser.each_well_and_parameters do |position, well_updates|
              # We might have a nil well if a plate was only partially cherrypicked
              well = well_hash[position] || next
              well_updates.each do |attribute, value|
                QcResult.create!(
      Severity: Minor
      Found in app/models/plate.rb by reek

      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.

      Plate#construct! refers to 'plate' more than self (maybe move it to another class?)
      Open

              plate
                .maps
                .in_row_major_order
                .ids
                .map { |location_id| { map_id: location_id } }
      Severity: Minor
      Found in app/models/plate.rb by reek

      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.

      Plate#stock_wells has approx 7 statements
      Open

        def stock_wells # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
      Severity: Minor
      Found in app/models/plate.rb by reek

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

      Plate#update_qc_values_with_parser has approx 7 statements
      Open

        def update_qc_values_with_parser(parser)
      Severity: Minor
      Found in app/models/plate.rb by reek

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

      Plate has at least 48 methods
      Open

      class Plate < Labware # rubocop:todo Metrics/ClassLength
      Severity: Minor
      Found in app/models/plate.rb by reek

      Too Many Methods is a special case of LargeClass.

      Example

      Given this configuration

      TooManyMethods:
        max_methods: 3

      and this code:

      class TooManyMethods
        def one; end
        def two; end
        def three; end
        def four; end
      end

      Reek would emit the following warning:

      test.rb -- 1 warning:
        [1]:TooManyMethods has at least 4 methods (TooManyMethods)

      Plate tests 'wells.loaded?' at least 3 times
      Open

          wells.loaded? ? wells.indexed_by_location[well_name] : wells.located_at_position(well_name).first
        end
        alias find_well_by_map_description find_well_by_name
      
        def plate_rows
      Severity: Minor
      Found in app/models/plate.rb by reek

      Repeated Conditional is a special case of Simulated Polymorphism. Basically it means you are checking the same value throughout a single class and take decisions based on this.

      Example

      Given

      class RepeatedConditionals
        attr_accessor :switch
      
        def repeat_1
          puts "Repeat 1!" if switch
        end
      
        def repeat_2
          puts "Repeat 2!" if switch
        end
      
        def repeat_3
          puts "Repeat 3!" if switch
        end
      end

      Reek would emit the following warning:

      test.rb -- 4 warnings:
        [5, 9, 13]:RepeatedConditionals tests switch at least 3 times (RepeatedConditional)

      If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction.

      Plate#construct! calls 'ids.map' 2 times
      Open

                  WellAttribute.import(ids.map { |well| { well_id: well } })
                  Uuid.import(
                    ids.map { |well| { resource_id: well, resource_type: well_type, external_id: Uuid.generate_uuid } }
      Severity: Minor
      Found in app/models/plate.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.

      Plate#construct! calls 'plate.wells' 2 times
      Open

                  plate.wells.import(wells)
                  ids = plate.wells.ids
      Severity: Minor
      Found in app/models/plate.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 Plate::search_for_plates (22.6)
      Open

        def self.search_for_plates(params)
          with_faculty_sponsor_ids(params[:faculty_sponsor_ids] || nil)
            .with_study_id(params[:study_id] || nil)
            .with_plate_purpose_ids(params[:plate_purpose_ids] || nil)
            .created_between(params[:start_date], params[:end_date])
      Severity: Minor
      Found in app/models/plate.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

      Plate has missing safe method 'construct!'
      Open

          def construct! # rubocop:todo Metrics/AbcSize
      Severity: Minor
      Found in app/models/plate.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.

      Plate#stock_plate? performs a nil-check
      Open

          return true if plate_purpose.nil?
      Severity: Minor
      Found in app/models/plate.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)

      Plate#set_plate_name_and_size performs a nil-check
      Open

          self.size = default_plate_size if size.nil?
      Severity: Minor
      Found in app/models/plate.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)

      Plate#stock_wells has the variable name 'w'
      Open

            wells.with_pool_id.index_with { |w| [w] }
          else
            wells
              .include_stock_wells
              .with_pool_id
      Severity: Minor
      Found in app/models/plate.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.

      TODO found
      Open

        # TODO: When on Ruby 2.6 try using endless ranges
      Severity: Minor
      Found in app/models/plate.rb by fixme

      There are no issues that match your filters.

      Category
      Status