3scale/porta

View on GitHub
app/lib/finance/builder/xml_markup.rb

Summary

Maintainability
A
1 hr
Test Coverage

Method invoice! has 33 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def invoice!(invoice)
    builder.invoice(:version => VERSION) do |xml|
      xml.id_ invoice.id
      xml.friendly_id invoice.friendly_id
      xml.creation_type invoice.creation_type
Severity: Minor
Found in app/lib/finance/builder/xml_markup.rb - About 1 hr to fix

    Finance::Builder::XmlMarkup#account! has approx 10 statements
    Open

      def account!(xml, name, account, address = nil)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.)

    Finance::Builder::XmlMarkup#self.invoices! contains iterators nested 2 deep
    Open

          items.each { |i| i.to_xml(:builder => xml ) }
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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)

    Finance::Builder::XmlMarkup#line_item! refers to 'item' more than self (maybe move it to another class?)
    Open

          xml.id_ item.id
          xml.name item.name
          xml.description item.description
          xml.quantity item.quantity
          xml.type item.type
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#invoice! refers to 'invoice' more than self (maybe move it to another class?)
    Open

          xml.id_ invoice.id
          xml.friendly_id invoice.friendly_id
          xml.creation_type invoice.creation_type
    
          xml.state invoice.state
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#invoice! has approx 24 statements
    Open

      def invoice!(invoice)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.)

    Finance::Builder::XmlMarkup#line_item! has approx 11 statements
    Open

      def line_item!(item)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.)

    Finance::Builder::XmlMarkup#account! has 4 parameters
    Open

      def account!(xml, name, account, address = nil)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.rb by reek

    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.

    Finance::Builder::XmlMarkup#invoice! contains iterators nested 2 deep
    Open

          xml.period do |xml|
            date!(xml, :from, invoice.period.begin)
            date!(xml, :to,   invoice.period.end)
          end
    
    
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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)

    Finance::Builder::XmlMarkup#invoice! calls 'invoice.vat_rate' 2 times
    Open

          if invoice.vat_rate.try!(:>, 0)
            xml.vat_rate       invoice.vat_rate
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#invoice! calls 'invoice.buyer_account' 2 times
    Open

          if invoice.buyer_account
            account!(xml, :buyer, invoice.buyer_account, invoice.to.presence) # presence is overloaded
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#invoice! calls 'invoice.period' 2 times
    Open

            date!(xml, :from, invoice.period.begin)
            date!(xml, :to,   invoice.period.end)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#line_item! manually dispatches method call
    Open

          xml.metric_id item.metric_id if item.respond_to?(:metric)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.rb by reek

    Reek reports a Manual Dispatch smell if it finds source code that manually checks whether an object responds to a method before that method is called. Manual dispatch is a type of Simulated Polymorphism which leads to code that is harder to reason about, debug, and refactor.

    Example

    class MyManualDispatcher
      attr_reader :foo
    
      def initialize(foo)
        @foo = foo
      end
    
      def call
        foo.bar if foo.respond_to?(:bar)
      end
    end

    Reek would emit the following warning:

    test.rb -- 1 warning:
      [9]: MyManualDispatcher manually dispatches method call (ManualDispatch)

    Finance::Builder::XmlMarkup has missing safe method 'date!'
    Open

      def date!(xml, name, value)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#account! doesn't depend on instance state (maybe move it to another class?)
    Open

      def account!(xml, name, account, address = nil)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.rb by reek

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

    Finance::Builder::XmlMarkup#date! doesn't depend on instance state (maybe move it to another class?)
    Open

      def date!(xml, name, value)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.rb by reek

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

    Finance::Builder::XmlMarkup has missing safe method 'account!'
    Open

      def account!(xml, name, account, address = nil)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup has missing safe method 'line_item!'
    Open

      def line_item!(item)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup has missing safe method 'invoice!'
    Open

      def invoice!(invoice)
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    Finance::Builder::XmlMarkup#self.invoices! has the variable name 'i'
    Open

          items.each { |i| i.to_xml(:builder => xml ) }
    Severity: Minor
    Found in app/lib/finance/builder/xml_markup.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.

    There are no issues that match your filters.

    Category
    Status