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

2294-2 #261

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

2294-2 #261

wants to merge 2 commits into from

Conversation

DenisVolchek
Copy link
Contributor

Номер

2294

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

2

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

https://youtu.be/tfh_3mSzRe8

Комментарии

Переписал "простыню"

puts table
end

def bad_words_searching(text)

Choose a reason for hiding this comment

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

UtilityFunction: TopBattlers#bad_words_searching doesn't depend on instance state (maybe move it to another class?). More info.

@@ -0,0 +1,7 @@
def pluralize(count, words)

Choose a reason for hiding this comment

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

UtilityFunction: pluralize doesn't depend on instance state (maybe move it to another class?). More info.

Copy link
Member

Choose a reason for hiding this comment

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

remove this file

words.last
end

def row

Choose a reason for hiding this comment

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

Metrics/AbcSize: Assignment Branch Condition size for row is too high. [18/15]

words.size / rounds
end

def pluralize(count, words)

Choose a reason for hiding this comment

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

UtilityFunction: Analyze#pluralize doesn't depend on instance state (maybe move it to another class?). More info.

@DenisVolchek DenisVolchek changed the title Write program in OOP. Add README.md 2294-2 Jul 18, 2018
# Analyze text of current artist

class Analyze
WORD_SETTING = [%w[слово слова слов], %w[нецензурное нецензурных нецензурных], %w[батл батла батлов]].freeze
Copy link
Member

Choose a reason for hiding this comment

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

move to yml.

end

def rounds
battles_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.

magic number

@@ -0,0 +1,7 @@
def pluralize(count, words)
Copy link
Member

Choose a reason for hiding this comment

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

remove this file


def bad_words_searching(text)
count = 0
RussianObscenity.find(text).each do |bad|
Copy link
Member

Choose a reason for hiding this comment

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

inject


def clean_text(name)
@hash[name].join(' ').split(' ').delete_if do |word|
word.length <= 2 || JUNK_WORDS.include?(word)
Copy link
Member

Choose a reason for hiding this comment

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

magic number


private


Choose a reason for hiding this comment

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

Layout/EmptyLines: Extra blank line detected.

@rapers = FileParser.new.read_files
@result = []
end

Choose a reason for hiding this comment

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

Layout/TrailingWhitespace: Trailing whitespace detected.

@anatoliliotych
Copy link
Member

why it doesn't match the task requirements?


RussianObscenity.dictionary = [:default, 'dictionary.yml']
# Form the table of top battlers with the biggest count of foul words
# :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.

why the whole class?

end

def row
[column_one, column_two, column_three, column_four, column_five]
Copy link
Member

Choose a reason for hiding this comment

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

strange naming...

RussianObscenity.dictionary = [:default, 'dictionary.yml']

# Analyze text of current artist
# :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.

remove this.


def bad_count
count = 0
RussianObscenity.find(text).each do |bad|
Copy link
Member

Choose a reason for hiding this comment

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

try inject

end

def column_three
"#{bad_count} #{pluralize(bad_count, WORD_SETTING['two'])} #{pluralize(bad_count, WORD_SETTING['one'])}"
Copy link
Member

Choose a reason for hiding this comment

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

these one/two/three - are unreadable

def sort_list
@rapers.sort_by do |_key, value|
bad_words_searching(value.join(' '))
end.reverse
Copy link
Member

Choose a reason for hiding this comment

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

reverse! is faster.

Copy link
Member

Choose a reason for hiding this comment

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

@DenisVolchek DenisVolchek force-pushed the 2294-2 branch 2 times, most recently from daee490 to 25a8d8c Compare July 23, 2018 06:29

def warning_about_wrong_name(name)
return if @hash.key?(name)
rapers_exampels = @hash.keys.sample
Copy link
Member

Choose a reason for hiding this comment

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

typo, please avoid typos in variable/method names - use spellcheckers

end

def clean_text(name)
@hash[name].join(' ').split(' ').delete_if do |word|
Copy link
Member

Choose a reason for hiding this comment

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

what is the sense to join and then split back?

2294/2/app.rb Outdated
require_relative './top_battlers.rb'
require_relative './top_words.rb'
require_relative './parameters_parser.rb'

Copy link
Member

Choose a reason for hiding this comment

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

why do you need this file? I thought that we agreed that it could be removed.

end

def parse(file)
File.open("./rap-battles/#{file}") do |file_name|
Copy link
Member

Choose a reason for hiding this comment

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

you can put your folder name to constant

battles_count * DEFAULT_BATTLES
end

def bad_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 should read about memoization: http://gavinmiller.io/2013/basics-of-ruby-memoization/

@name = file.split(/против|aka|vs\b/i).first.strip
parse(file)
end
@rapers
Copy link
Member

Choose a reason for hiding this comment

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

this return is really unexpected


def sort_list
@rapers.sort_by do |_key, value|
@text = value.join(' ')
Copy link
Member

Choose a reason for hiding this comment

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

avoid using instance variables for such cases, were you need just pass reek.

end
end

def read_files
Copy link
Member

Choose a reason for hiding this comment

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

leave only this method as public

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