basestylo/Tweakphoeus

View on GitHub

Showing 61 of 61 total issues

Tweakphoeus::Client#initialize has boolean parameter 'redirect'
Open

    def initialize(ua_systems: %i[linux mac], ssl_verifypeer: true, redirect: false)
Severity: Minor
Found in lib/tweakphoeus.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

Tweakphoeus::CookieJar#parse_cookie has approx 6 statements
Open

    def parse_cookie(string)
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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.)

Tweakphoeus::Client has at least 7 instance variables
Open

  class Client
Severity: Minor
Found in lib/tweakphoeus.rb by reek

Too Many Instance Variables is a special case of LargeClass.

Example

Given this configuration

TooManyInstanceVariables:
  max_instance_variables: 3

and this code:

class TooManyInstanceVariables
  def initialize
    @arg_1 = :dummy
    @arg_2 = :dummy
    @arg_3 = :dummy
    @arg_4 = :dummy
  end
end

Reek would emit the following warning:

test.rb -- 5 warnings:
  [1]:TooManyInstanceVariables has at least 4 instance variables (TooManyInstanceVariables)

Tweakphoeus::Client#initialize has boolean parameter 'ssl_verifypeer'
Open

    def initialize(ua_systems: %i[linux mac], ssl_verifypeer: true, redirect: false)
Severity: Minor
Found in lib/tweakphoeus.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

Tweakphoeus::CookieJar#cookie_string has approx 8 statements
Open

    def cookie_string(url, headers = {})
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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.)

Tweakphoeus::RefererList#referer_from_headers refers to 'headers' more than self (maybe move it to another class?)
Open

      @referer.last.replace(headers['Referer']) if headers && headers['Referer'].is_a?(String)
Severity: Minor
Found in lib/tweakphoeus/referer_list.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.

Tweakphoeus::CookieJar#obtain_cookies has approx 6 statements
Open

    def obtain_cookies(response)
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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.)

Tweakphoeus::Client#get has 4 parameters
Open

    def get(url, body: nil, params: nil, headers: nil)
Severity: Minor
Found in lib/tweakphoeus.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.

Tweakphoeus::Client#http_request has 5 parameters
Open

    def http_request(url, body: nil, params: nil, headers: nil, method: :get)
Severity: Minor
Found in lib/tweakphoeus.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.

Complex method Tweakphoeus::CookieJar#obtain_cookies (23.9)
Open

    def obtain_cookies(response)
      set_cookies_field = [response.headers['Set-Cookie']].compact

      set_cookies_field.each do |cookie|
        key, value = cookie.match(/^([^=]+)=(.+)/).to_a[1..]
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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

Tweakphoeus::CookieJar#set_cookie calls '@cookie_jar[domain]' 3 times
Open

      @cookie_jar[domain] ||= {}

      value = value.split(';').first

      if value == '""'
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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.

Tweakphoeus::RefererList#referer_from_headers calls 'headers['Referer']' 2 times
Open

      @referer.last.replace(headers['Referer']) if headers && headers['Referer'].is_a?(String)
Severity: Minor
Found in lib/tweakphoeus/referer_list.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.

Tweakphoeus::CookieJar#add_cookies calls '@cookie_jar[domain]' 2 times
Open

      @cookie_jar[domain] ||= {}
      @cookie_jar[domain][key] = value
Severity: Minor
Found in lib/tweakphoeus/cookie_jar.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.

Tweakphoeus::UserAgent#browser calls 'Random.rand(0..9)' 2 times
Open

        webkit603 = "603.#{Random.rand(0..9)}.#{Random.rand(0..9)}"
Severity: Minor
Found in lib/tweakphoeus/user_agent.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.

Tweakphoeus::Client#redirect_url doesn't depend on instance state (maybe move it to another class?)
Open

    def redirect_url(response)
Severity: Minor
Found in lib/tweakphoeus.rb by reek

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

Tweakphoeus::Client#base_headers is a writable attribute
Open

    attr_accessor :cookie_jar, :referer_list, :base_headers, :redirect
Severity: Minor
Found in lib/tweakphoeus.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

Tweakphoeus::Client#redirect? performs a nil-check
Open

      !redirect_url(response).nil?
Severity: Minor
Found in lib/tweakphoeus.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)

Tweakphoeus::Client#build_base_headers doesn't depend on instance state (maybe move it to another class?)
Open

    def build_base_headers(ua_systems)
Severity: Minor
Found in lib/tweakphoeus.rb by reek

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

Tweakphoeus::Helpers#get_domain doesn't depend on instance state (maybe move it to another class?)
Open

    def get_domain(domain)
Severity: Minor
Found in lib/tweakphoeus/helpers.rb by reek

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

Tweakphoeus::Client#cookie_jar is a writable attribute
Open

    attr_accessor :cookie_jar, :referer_list, :base_headers, :redirect
Severity: Minor
Found in lib/tweakphoeus.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)
Severity
Category
Status
Source
Language