Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

2355 - 2 #243

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

2355 - 2 #243

wants to merge 8 commits into from

Conversation

Aleksei12942
Copy link
Contributor

@Aleksei12942 Aleksei12942 commented Jul 16, 2018

Номер

2355

Номер задания

2

Ссылка на видео с демо

https://youtu.be/wehCfIbeW28

Комментарии

Успел сделать 1 и 2 уровни, но вот как делать третий так и не придумал. Есть одна сложнореализуемая идея, но она требует больше времени.

@top_words = {}
end

# This method smells of :reek:NestedIterators

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layout/CommentIndentation: Incorrect indentation detected (column 1 instead of 2).

def average_bad_words_in_battle(battler)
top_obscenity["#{battler}"] / (Dir[File.join("./rap-battles/#{battler}/", '**', '*')].count { |file| File.file?(file) })
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: unexpected token kEND

# This method smells of :reek:DuplicateMethodCall
def set_top_obscenity
0.upto(battlers.size - 1) do |index
check = FindObscenity.new(@battlers[indexx])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: unexpected token tIDENTIFIER

# rubocop:disable Lint/Syntax
# This class is needed for first level of Task 2
# This class smells of :reek:Attribute
class TopBad

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: class definition in method body

# This method smells of :reek:DuplicateMethodCall
def set_top_obscenity
0.upto(battlers.size - 1) do |index
check = FindObscenity.new(@battlers[indexx])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has unexpected token tIDENTIFIER. More info.

def average_bad_words_in_battle(battler)
top_obscenity["#{battler}"] / (Dir[File.join("./rap-battles/#{battler}/", '**', '*')].count { |file| File.file?(file) })
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has unexpected token kEND. More info.

# rubocop:disable Lint/Syntax
# This class is needed for first level of Task 2
# This class smells of :reek:Attribute
class TopBad

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has class definition in method body. More info.

def average_bad_words_in_battle(battler)
top_obscenity["#{battler}"] / (Dir[File.join("./rap-battles/#{battler}/", '**', '*')].count { |file| File.file?(file) })
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: unexpected token kEND

# This method smells of :reek:DuplicateMethodCall
def set_top_obscenity
0.upto(battlers.size - 1) do |index
check = FindObscenity.new(@battlers[indexx])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: unexpected token tIDENTIFIER

# rubocop:disable Lint/Syntax
# This class is needed for first level of Task 2
# This class smells of :reek:Attribute
class TopBad

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: class definition in method body

# This method smells of :reek:DuplicateMethodCall
def set_top_obscenity
0.upto(battlers.size - 1) do |index
check = FindObscenity.new(@battlers[indexx])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has unexpected token tIDENTIFIER. More info.

def average_bad_words_in_battle(battler)
top_obscenity["#{battler}"] / (Dir[File.join("./rap-battles/#{battler}/", '**', '*')].count { |file| File.file?(file) })
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has unexpected token kEND. More info.

# rubocop:disable Lint/Syntax
# This class is needed for first level of Task 2
# This class smells of :reek:Attribute
class TopBad

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax: This file has class definition in method body. More info.

@@ -0,0 +1,50 @@
require './find_obscenity.rb'

# rubocop:disable Lint/UnusedBlockArgument

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/UnneededCopDisableDirective: Unnecessary disabling of Lint/UnusedBlockArgument.

# rubocop:enable Metrics/AbcSize
# rubocop:enable Style/IfUnlessModifier
# rubocop:enable Lint/ImplicitStringConcatenation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layout/TrailingBlankLines: 1 trailing blank lines detected.

@anatoliliotych
Copy link
Member

Please, fix commit history, removing files with another commit is useless.

@@ -0,0 +1,58 @@
require 'russian_obscenity'

# rubocop:disable Metrics/MethodLength
Copy link
Member

@anatoliliotych anatoliliotych Jul 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these checks should be removed.

require 'russian_obscenity'

# This class is needed to find and collect all obscenity from text files
class FindObscenity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't name classes with verbs.

2355/2/main.rb Outdated
else
1
end
top = TopBad.new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't look like OOP. you've just moved all methods to class and calling them.

2355/2/main.rb Outdated
end
end
end.parse!
# rubocop:enable Metrics/BlockLength
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

end
end

# This method smells of :reek:DuplicateMethodCall
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

end
end

# This method smells of :reek:DuplicateMethodCall
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

file.each { |line| @battlers << line.delete("\n") }
end

# This method smells of :reek:UtilityFunction
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

2355/2/main.rb Outdated
require 'optparse'
require 'terminal-table'

# rubocop:disable Metrics/BlockLength
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

2355/2/main.rb Outdated
require 'terminal-table'

# rubocop:disable Metrics/BlockLength
# This disable is needed because this block is the main logic of the program.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment means that you just wanted to pass, but not to solve it. Please look through all videos about OOP we had. You should change 'the main logic of the program'

end
end

# This method smells of :reek:NestedIterators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

File.new('./mistakes').each { |line| @mistakes << line.delete("\n") }
end

# This method smells of :reek:NestedIterators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf?

2355/2/parser.rb Outdated
@top.average_words_in_round(elem).to_s
end

def print_table

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TooManyStatements: Parser#print_table has approx 6 statements. More info.

2355/2/parser.rb Outdated

def print_table
table = Terminal::Table.new do |tb|
@bad_words.times do |index|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NestedIterators: Parser#print_table contains iterators nested 2 deep. More info.

2355/2/parser.rb Outdated
require 'terminal-table'

# This class is needed to parsing console params
class Parser

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InstanceVariableAssumption: Parser assumes too much for instance variable '@bad_words'. More info.
InstanceVariableAssumption: Parser assumes too much for instance variable '@name'. More info.
TooManyInstanceVariables: Parser has at least 5 instance variables. More info.

2355/2/parser.rb Outdated
end

def top_words=(most)
@top_words = !most.empty? ? most.to_i : 30

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FeatureEnvy: Parser#top_words= refers to 'most' more than self (maybe move it to another class?). More info.

2355/2/parser.rb Outdated

def print_top_words
t_w = TopWord.new(@name)
t_w.check_all_words

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FeatureEnvy: Parser#print_top_words refers to 't_w' more than self (maybe move it to another class?). More info.

2355/2/parser.rb Outdated
end

def bad_words=(bad)
@bad_words = !bad.empty? ? bad.to_i : 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FeatureEnvy: Parser#bad_words= refers to 'bad' more than self (maybe move it to another class?). More info.

@anatoliliotych
Copy link
Member

please, don't leave your thoughts and jokes in commit messages.

@anatoliliotych
Copy link
Member

it's your personal task to write each commit message to fit these rules: https://chris.beams.io/posts/git-commit/

Copy link
Member

@anatoliliotych anatoliliotych left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, look through Lomov's explanation about how to structure the application in terms of OOP.

@words_count = 0
end

def battlers_names
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain logic of this method?

1.upto(@dir_count) do |text|
words_in_text(battler, text)
end
@words_count / (@dir_count * 3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove magic number

@top_words = {}
end

def dir_count
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have code duplication between two classes.

2355/2/README.md Outdated
@@ -0,0 +1,54 @@
# Инструкция по применению

Запустить в консоли файл __main.rb__ командой __ruby main.rb__, добавив в конце записи необходимые команды.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you read task requirements? how should I guess where to put texts of battles?

# I would have each block do-end to make a separate function and call them all one by one,
# but in my opinion, this will lower the readability of the code.
# This method smells of :reek:NestedIterators
def first_check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix all reek smells.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants