sanger/sequencescape

View on GitHub
app/models/receptacle.rb

Summary

Maintainability
B
6 hrs
Test Coverage
A
93%

Class Receptacle has 30 methods (exceeds 20 allowed). Consider refactoring.
Open

class Receptacle < Asset # rubocop:todo Metrics/ClassLength
  include Uuid::Uuidable
  include Commentable
  include Transfer::State
  include Aliquot::Remover
Severity: Minor
Found in app/models/receptacle.rb - About 3 hrs to fix

    File receptacle.rb has 251 lines of code (exceeds 250 allowed). Consider refactoring.
    Open

    class Receptacle < Asset # rubocop:todo Metrics/ClassLength
      include Uuid::Uuidable
      include Commentable
      include Transfer::State
      include Aliquot::Remover
    Severity: Minor
    Found in app/models/receptacle.rb - About 2 hrs to fix

      Receptacle#set_external_release has approx 7 statements
      Open

        def set_external_release(state) # rubocop:todo Metrics/MethodLength
      Severity: Minor
      Found in app/models/receptacle.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.)

      Receptacle has at least 30 methods
      Open

      class Receptacle < Asset # rubocop:todo Metrics/ClassLength
      Severity: Minor
      Found in app/models/receptacle.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)

      Receptacle#set_as_library has boolean parameter 'force'
      Open

        def set_as_library(force: false)
      Severity: Minor
      Found in app/models/receptacle.rb by reek

      Boolean Parameter is a special case of Control Couple, where a method parameter is defaulted to true or false. A Boolean Parameter effectively permits a method's caller to decide which execution path to take. This is a case of bad cohesion. You're creating a dependency between methods that is not really necessary, thus increasing coupling.

      Example

      Given

      class Dummy
        def hit_the_switch(switch = true)
          if switch
            puts 'Hitting the switch'
            # do other things...
          else
            puts 'Not hitting the switch'
            # do other things...
          end
        end
      end

      Reek would emit the following warning:

      test.rb -- 3 warnings:
        [1]:Dummy#hit_the_switch has boolean parameter 'switch' (BooleanParameter)
        [2]:Dummy#hit_the_switch is controlled by argument switch (ControlParameter)

      Note that both smells are reported, Boolean Parameter and Control Parameter.

      Getting rid of the smell

      This is highly dependent on your exact architecture, but looking at the example above what you could do is:

      • Move everything in the if branch into a separate method
      • Move everything in the else branch into a separate method
      • Get rid of the hit_the_switch method alltogether
      • Make the decision what method to call in the initial caller of hit_the_switch

      Receptacle#tag_range refers to 'map_ids' more than self (maybe move it to another class?)
      Open

          case map_ids.length
          when 0
            nil
          when 1
            map_ids.first
      Severity: Minor
      Found in app/models/receptacle.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.

      Receptacle#assign_tag2 refers to 'aliquot' more than self (maybe move it to another class?)
      Open

            aliquot.tag2 = tag
            aliquot.save!
      Severity: Minor
      Found in app/models/receptacle.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.

      Receptacle#set_as_library refers to 'aliquot' more than self (maybe move it to another class?)
      Open

            aliquot.set_library(force: force)
            aliquot.save!
      Severity: Minor
      Found in app/models/receptacle.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.

      Method set_external_release has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
      Open

        def set_external_release(state) # rubocop:todo Metrics/MethodLength
          update_external_release do
            if state == 'failed'
              self.external_release = false
            elsif state == 'passed'
      Severity: Minor
      Found in app/models/receptacle.rb - About 35 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

      Receptacle#tag_range calls 'map_ids.first' 2 times
      Open

            map_ids.first
          else
            "#{map_ids.first}-#{map_ids.last}"
      Severity: Minor
      Found in app/models/receptacle.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.

      Receptacle#update_external_release calls 'external_release.nil?' 2 times
      Open

          external_release_nil_before = external_release.nil?
          yield
          save!
          events.create_external_release!(!external_release_nil_before) unless external_release.nil?
      Severity: Minor
      Found in app/models/receptacle.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.

      Receptacle#set_external_release performs a nil-check
      Open

            elsif state.nil?
      Severity: Minor
      Found in app/models/receptacle.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)

      Receptacle#update_external_release performs a nil-check
      Open

          external_release_nil_before = external_release.nil?
          yield
          save!
          events.create_external_release!(!external_release_nil_before) unless external_release.nil?
      Severity: Minor
      Found in app/models/receptacle.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)

      Receptacle#assign_tag2 has the name 'assign_tag2'
      Open

        def assign_tag2(tag)
      Severity: Minor
      Found in app/models/receptacle.rb by reek

      An Uncommunicative Method Name is a method 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.

      Receptacle#update_aliquot_quality has the variable name 'a'
      Open

          aliquots.each { |a| a.update_quality(suboptimal_quality) }
      Severity: Minor
      Found in app/models/receptacle.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.

      Receptacle#attach_tag has the parameter name 'tag2'
      Open

        def attach_tag(tag, tag2 = nil)
      Severity: Minor
      Found in app/models/receptacle.rb by reek

      An Uncommunicative Parameter Name is a parameter 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.

      Receptacle#attach_tag has the variable name 'a'
      Open

          aliquots.each { |a| a.update!(tags) }
      Severity: Minor
      Found in app/models/receptacle.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

              self # TODO: Ignore for the moment, correct later
      Severity: Minor
      Found in app/models/receptacle.rb by fixme

      TODO found
      Open

              self # TODO: Ignore for the moment, correct later
      Severity: Minor
      Found in app/models/receptacle.rb by fixme

      There are no issues that match your filters.

      Category
      Status