-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Roman Androsiuk - 0 #33
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
Error: We found some problems with your configuration file: [/IrresponsibleMo...
Error: We found some problems with your configuration file: [/IrresponsibleModule] key 'IrresponsibleModule:' is undefined., [/DuplicateMethodCall] key 'DuplicateMethodCall:' is undefined., [/UtilityFunction] key 'UtilityFunction:' is undefined.
same_price_find | ||
same_price_output | ||
end | ||
#~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LeadingCommentSpace: Missing space after #.
@same_price_names << @sheetnow.cell(i + 8, 1) | ||
end | ||
end | ||
#~~~~~~~~~~~~~~~~~~~~~Main~code~~~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LeadingCommentSpace: Missing space after #.
(0..354).each do |i| | ||
next unless [email protected](i + 8, 1).nil? && [email protected](i + 8, 15).nil? | ||
|
||
next unless @sheetnow.cell(i + 8, 15) == @good_price && @sheetnow.cell(i + 8, 1) != @good_name # Item costs the same but it isn't itself |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/LineLength: Line is too long. [140/120]
puts answer # Prints answer | ||
end | ||
|
||
def same_price_find |
There was a problem hiding this comment.
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 same_price_find is too high. [18.47/15]
# Prints list of items with the same cost | ||
answer = 'You can\'t afford anything else for that price.' # Default answer | ||
same_length = @same_price_names.size | ||
if same_length > 0 # Changes answer if anything was found |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/NumericPredicate: Use same_length.positive? instead of same_length > 0.
def prices_find # Looking through all files for min and max price of the item | ||
@allfiles = Dir.entries('./data') | ||
allfiles_length = @allfiles.size | ||
(0...allfiles_length).each do |i0| # Going through all files in ./data/ directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/BlockLength: Block has too many lines. [26/25]
date # Returning a string | ||
end | ||
|
||
def prices_find # Looking through all files for min and max price of the item |
There was a problem hiding this comment.
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 prices_find is too high. [50.72/15]
Metrics/CyclomaticComplexity: Cyclomatic complexity for prices_find is too high. [11/6]
Metrics/MethodLength: Method has too many lines. [32/10]
Metrics/PerceivedComplexity: Perceived complexity for prices_find is too high. [12/7]
Style/CommentedKeyword: Do not place comments on the same line as the def keyword.
date += '2015' | ||
elsif date_text.include? '2017' | ||
date += '2017' | ||
@notbyn = false # Case to convert BYR to BYN cause all files before year 2017 had prices written in 'before denomination' format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/LineLength: Line is too long. [132/120]
end | ||
|
||
def date_read(sheet) | ||
# Read exact cell from requested sheet and generate mm/yyyy date |
There was a problem hiding this comment.
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 13 instead of 2).
end | ||
end | ||
|
||
def date_read(sheet) |
There was a problem hiding this comment.
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 date_read is too high. [59.27/15]
Metrics/CyclomaticComplexity: Cyclomatic complexity for date_read is too high. [23/6]
Metrics/MethodLength: Method has too many lines. [56/10]
Metrics/PerceivedComplexity: Perceived complexity for date_read is too high. [26/7]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap your code into a class
require 'roo' | ||
require 'roo-xls' | ||
# require 'rubocop' | ||
xlfilenow = Roo::Excelx.new('./data/Average_prices(serv)-10-2018.xlsx') # Info about prices in Minsk these days |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put code that is actually executed at the end of the file. Also you can put it all in main method and call it at the end of a file.
require 'roo-xls' | ||
# require 'rubocop' | ||
xlfilenow = Roo::Excelx.new('./data/Average_prices(serv)-10-2018.xlsx') # Info about prices in Minsk these days | ||
@sheetnow = xlfilenow.sheet(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to use instance variables outside of any class
# Read exact cell from requested sheet and generate mm/yyyy date | ||
date = '' # Actually need a tip how to make this not so dumb | ||
date_text = sheet.cell(3, 1) | ||
if date_text.include? 'январь' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use hash to map month names to dates.
Name
Roman Androsiuk
Homework#
0
Comment
Levels 1, 2 and 3