countries/country_select

View on GitHub

Showing 98 of 99 total issues

Assignment Branch Condition size for country_option_tags is too high. [33.66/15]
Open

    def country_option_tags
      # In Rails 5.2+, `value` accepts no arguments and must also be called
      # with parens to avoid the local variable of the same name
      # https://github.com/rails/rails/pull/29791
      selected_option = @options.fetch(:selected) do
Severity: Minor
Found in lib/country_select/tag_helper.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Method has too many lines. [24/10]
Open

    def country_options_for(country_codes, sorted=true)
      I18n.with_locale(locale) do
        country_list = country_codes.map do |code_or_name|
          if country = ISO3166::Country.new(code_or_name)
            code = country.alpha2
Severity: Minor
Found in lib/country_select/tag_helper.rb by rubocop

This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.

Method has too many lines. [21/10]
Open

    def country_option_tags
      # In Rails 5.2+, `value` accepts no arguments and must also be called
      # with parens to avoid the local variable of the same name
      # https://github.com/rails/rails/pull/29791
      selected_option = @options.fetch(:selected) do
Severity: Minor
Found in lib/country_select/tag_helper.rb by rubocop

This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.

Complex method CountrySelect::TagHelper#country_option_tags (50.5)
Open

    def country_option_tags
      # In Rails 5.2+, `value` accepts no arguments and must also be called
      # with parens to avoid the local variable of the same name
      # https://github.com/rails/rails/pull/29791
      selected_option = @options.fetch(:selected) do
Severity: Minor
Found in lib/country_select/tag_helper.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

Assignment Branch Condition size for country_options_for is too high. [18.17/15]
Open

    def country_options_for(country_codes, sorted=true)
      I18n.with_locale(locale) do
        country_list = country_codes.map do |code_or_name|
          if country = ISO3166::Country.new(code_or_name)
            code = country.alpha2
Severity: Minor
Found in lib/country_select/tag_helper.rb by rubocop

This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See http://c2.com/cgi/wiki?AbcMetric

Method country_options_for has a Cognitive Complexity of 15 (exceeds 5 allowed). Consider refactoring.
Open

    def country_options_for(country_codes, sorted=true)
      I18n.with_locale(locale) do
        country_list = country_codes.map do |code_or_name|
          if country = ISO3166::Country.new(code_or_name)
            code = country.alpha2
Severity: Minor
Found in lib/country_select/tag_helper.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

Perceived complexity for country_options_for is too high. [9/7]
Open

    def country_options_for(country_codes, sorted=true)
      I18n.with_locale(locale) do
        country_list = country_codes.map do |code_or_name|
          if country = ISO3166::Country.new(code_or_name)
            code = country.alpha2
Severity: Minor
Found in lib/country_select/tag_helper.rb by rubocop

This cop tries to produce a complexity score that's a measure of the complexity the reader experiences when looking at a method. For that reason it considers when nodes as something that doesn't add as much complexity as an if or a &&. Except if it's one of those special case/when constructs where there's no expression after case. Then the cop treats it as an if/elsif/elsif... and lets all the when nodes count. In contrast to the CyclomaticComplexity cop, this cop considers else nodes as adding complexity.

Example:

def my_method                   # 1
  if cond                       # 1
    case var                    # 2 (0.8 + 4 * 0.2, rounded)
    when 1 then func_one
    when 2 then func_two
    when 3 then func_three
    when 4..10 then func_other
    end
  else                          # 1
    do_something until a && b   # 2
  end                           # ===
end                             # 7 complexity points

Method has too many lines. [12/10]
Open

      def country_select(method, priority_or_options = {}, options = {}, html_options = {})
        if Hash === priority_or_options
          html_options = options
          options = priority_or_options
        else

This cop checks if the length of a method exceeds some maximum value. Comment lines can optionally be ignored. The maximum allowed length is configurable.

Complex method CountrySelect::TagHelper#country_options_for (27.7)
Open

    def country_options_for(country_codes, sorted=true)
      I18n.with_locale(locale) do
        country_list = country_codes.map do |code_or_name|
          if country = ISO3166::Country.new(code_or_name)
            code = country.alpha2
Severity: Minor
Found in lib/country_select/tag_helper.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

CountrySelect::TagHelper#country_options_for has boolean parameter 'sorted'
Open

    def country_options_for(country_codes, sorted=true)
Severity: Minor
Found in lib/country_select/tag_helper.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

ActionView::Helpers::FormBuilder#country_select has approx 6 statements
Open

      def country_select(method, priority_or_options = {}, options = {}, html_options = {})

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

ActionView::Helpers::FormOptionsHelper#country_select has 4 parameters
Open

      def country_select(object, method, options = {}, html_options = {})

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.

ActionView::Helpers::FormBuilder#country_select has 4 parameters
Open

      def country_select(method, priority_or_options = {}, options = {}, html_options = {})

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.

CountrySelect::TagHelper#country_options_for has approx 12 statements
Open

    def country_options_for(country_codes, sorted=true)
Severity: Minor
Found in lib/country_select/tag_helper.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.)

CountrySelect::TagHelper#country_options_for is controlled by argument 'sorted'
Open

        if sorted
Severity: Minor
Found in lib/country_select/tag_helper.rb by reek

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

CountrySelect::TagHelper#country_option_tags has approx 12 statements
Open

    def country_option_tags
Severity: Minor
Found in lib/country_select/tag_helper.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.)

Method country_option_tags has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
Open

    def country_option_tags
      # In Rails 5.2+, `value` accepts no arguments and must also be called
      # with parens to avoid the local variable of the same name
      # https://github.com/rails/rails/pull/29791
      selected_option = @options.fetch(:selected) do
Severity: Minor
Found in lib/country_select/tag_helper.rb - About 45 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

ActionView::Helpers::FormBuilder assumes too much for instance variable '@default_options'
Open

    class FormBuilder

Classes should not assume that instance variables are set or present outside of the current class definition.

Good:

class Foo
  def initialize
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Good as well:

class Foo
  def foo?
    bar == :foo
  end

  def bar
    @bar ||= :foo
  end
end

Bad:

class Foo
  def go_foo!
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Example

Running Reek on:

class Dummy
  def test
    @ivar
  end
end

would report:

[1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar

Note that this example would trigger this smell warning as well:

class Parent
  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    @omg
  end
end

The way to address the smell warning is that you should create an attr_reader to use @omg in the subclass and not access @omg directly like this:

class Parent
  attr_reader :omg

  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    omg
  end
end

Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.

If you don't want to expose those methods as public API just make them private like this:

class Parent
  def initialize(omg)
    @omg = omg
  end

  private
  attr_reader :omg
end

class Child < Parent
  def foo
    omg
  end
end

Current Support in Reek

An instance variable must:

  • be set in the constructor
  • or be accessed through a method with lazy initialization / memoization.

If not, Instance Variable Assumption will be reported.

ActionView::Helpers::FormBuilder assumes too much for instance variable '@object_name'
Open

    class FormBuilder

Classes should not assume that instance variables are set or present outside of the current class definition.

Good:

class Foo
  def initialize
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Good as well:

class Foo
  def foo?
    bar == :foo
  end

  def bar
    @bar ||= :foo
  end
end

Bad:

class Foo
  def go_foo!
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Example

Running Reek on:

class Dummy
  def test
    @ivar
  end
end

would report:

[1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar

Note that this example would trigger this smell warning as well:

class Parent
  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    @omg
  end
end

The way to address the smell warning is that you should create an attr_reader to use @omg in the subclass and not access @omg directly like this:

class Parent
  attr_reader :omg

  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    omg
  end
end

Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.

If you don't want to expose those methods as public API just make them private like this:

class Parent
  def initialize(omg)
    @omg = omg
  end

  private
  attr_reader :omg
end

class Child < Parent
  def foo
    omg
  end
end

Current Support in Reek

An instance variable must:

  • be set in the constructor
  • or be accessed through a method with lazy initialization / memoization.

If not, Instance Variable Assumption will be reported.

CountrySelect::CountryNotFoundError has no descriptive comment
Open

  class CountryNotFoundError < StandardError;end
Severity: Minor
Found in lib/country_select/tag_helper.rb by reek

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
Severity
Category
Status
Source
Language