Sign upLogin

Trevoke/SGFParser

View on GitHub
Star

Showing 171 of 171 total issues

SGF::Gametree has no descriptive comment
Open

  class Gametree
Severity: Minor
Found in lib/sgf/properties.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

SGF::NoIdentityError has no descriptive comment
Open

  class NoIdentityError < StandardError
Severity: Minor
Found in lib/sgf/error.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

SGF::Parser assumes too much for instance variable '@sgf_stream'
Open

class SGF::Parser
Severity: Minor
Found in lib/sgf/parser.rb by reek

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.

SGF::GenericPropertyToken has no descriptive comment
Open

class SGF::GenericPropertyToken
Severity: Minor
Found in lib/sgf/parsing_tokens.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

SGF::Collection#gametrees is a writable attribute
Open

    attr_accessor :current_node, :errors, :gametrees
Severity: Minor
Found in lib/sgf/collection.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)

SGF::Collection#current_node is a writable attribute
Open

    attr_accessor :current_node, :errors, :gametrees
Severity: Minor
Found in lib/sgf/collection.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)

SGF::Parser#property_token_type doesn't depend on instance state (maybe move it to another class?)
Open

  def property_token_type(identity)
Severity: Minor
Found in lib/sgf/parser.rb by reek

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

SGF::CommentToken#transform doesn't depend on instance state (maybe move it to another class?)
Open

  def transform(token)
Severity: Minor
Found in lib/sgf/parsing_tokens.rb by reek

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

SGF::Node#flexible doesn't depend on instance state (maybe move it to another class?)
Open

  def flexible(id)
Severity: Minor
Found in lib/sgf/node.rb by reek

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

SGF::GenericPropertyToken#still_inside? doesn't depend on instance state (maybe move it to another class?)
Open

  def still_inside?(char, _token_so_far, _sgf_stream)
Severity: Minor
Found in lib/sgf/parsing_tokens.rb by reek

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

SGF::Gametree#current_node is a writable attribute
Open

    attr_accessor :current_node
Severity: Minor
Found in lib/sgf/gametree.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)

Method peek_skipping_whitespace has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

  def peek_skipping_whitespace
    while char = next_character
      next if char[/\s/]

      break
Severity: Minor
Found in lib/sgf/stream.rb - About 25 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

SGF::MultiPropertyToken#transform doesn't depend on instance state (maybe move it to another class?)
Open

  def transform(token)
Severity: Minor
Found in lib/sgf/parsing_tokens.rb by reek

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

SGF::GtpWriter#upside_down is a writable attribute
Open

  attr_writer :upside_down
Severity: Minor
Found in lib/sgf/gtp_writer.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)

SGF::Node#children is a writable attribute
Open

  attr_accessor :children, :properties
Severity: Minor
Found in lib/sgf/node.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)

Method gtp_move has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

  def gtp_move(node)
    pps = node.properties
    if pps['SZ']
      @boardsize = pps['SZ'].to_i
      out = []
Severity: Minor
Found in lib/sgf/gtp_writer.rb - About 25 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

SGF::IdentityToken#transform doesn't depend on instance state (maybe move it to another class?)
Open

  def transform(token)
Severity: Minor
Found in lib/sgf/parsing_tokens.rb by reek

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

Method parse has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

  def parse(sgf, strict_parsing = true)
    error_checker = strict_parsing ? SGF::StrictErrorChecker.new : SGF::LaxErrorChecker.new
    @sgf_stream = SGF::Stream.new(sgf, error_checker)
    @assembler = SGF::CollectionAssembler.new
    until @sgf_stream.eof?
Severity: Minor
Found in lib/sgf/parser.rb - About 25 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

SGF::MultiPropertyToken#still_inside? doesn't depend on instance state (maybe move it to another class?)
Open

  def still_inside?(char, _token_so_far, sgf_stream)
Severity: Minor
Found in lib/sgf/parsing_tokens.rb by reek

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

SGF::Stream#clean doesn't depend on instance state (maybe move it to another class?)
Open

  def clean(sgf)
Severity: Minor
Found in lib/sgf/stream.rb by reek

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

Severity
Category
Status
Source
Language