2199 - 3 #277
2199 - 3 #277
Conversation
2199/3/app/onliner_page_parser.rb
Outdated
| @browser ||= Capybara.current_session | ||
| end | ||
|
|
||
| def visit_page |
There was a problem hiding this comment.
TooManyStatements: OnlinerPageParser#visit_page has approx 7 statements. More info.
2199/3/app/comment_analyzer.rb
Outdated
| @texts = texts | ||
| end | ||
|
|
||
| def analyze |
There was a problem hiding this comment.
Metrics/AbcSize: Assignment Branch Condition size for analyze is too high. [16.03/15]
2199/3/app/comment_analyzer.rb
Outdated
|
|
||
| def analyze | ||
| JSON.parse(run_request.body)['documents'].each_with_object([]) do |result, store| | ||
| document = documents.find { |doc| doc[:id].to_s == result['id'] } |
There was a problem hiding this comment.
NestedIterators: CommentAnalyzer#analyze contains iterators nested 2 deep. More info.
2199/3/app/comment_analyzer.rb
Outdated
|
|
||
| def request | ||
| request = Net::HTTP::Post.new(endpoint) | ||
| request['Content-Type'] = 'application/json' |
There was a problem hiding this comment.
FeatureEnvy: CommentAnalyzer#request refers to 'request' more than self (maybe move it to another class?). More info.
|
Stick to RESTful routes. Example: |
2199/3/app/comment_analyzer.rb
Outdated
| # analyze comment | ||
| class CommentAnalyzer | ||
| ACCESS_KEY = ENV['ACCESS_KEY'] | ||
| AZURE_ENDPOINT = ENV['AZURE_ENDPOINT'] |
There was a problem hiding this comment.
Does Azure endpoint contain responsive information?
If no, why should we store it in .ENV variable?
| document = documents.find { |doc| doc[:id].to_s == result['id'] } | ||
| store << { | ||
| text: document[:text], | ||
| rating: result['score'] * 200 - 100 |
There was a problem hiding this comment.
The formula is too magical for me (where do the numbers (*200 - 100 ) come from?)
Let's move it to separate method and describe a little bit.
There was a problem hiding this comment.
We get a decimal number and this formula needed to lead it to necessary range
2199/3/app/comment_analyzer.rb
Outdated
|
|
||
| # :reek:FeatureEnvy | ||
| def request | ||
| request = Net::HTTP::Post.new(endpoint) |
There was a problem hiding this comment.
You can use #tap.
Net::HTTP::Post.new(endpoint).tap do |request|
request['Content-Type'] = 'application/json'
request['Ocp-Apim-Subscription-Key'] = ACCESS_KEY
request.body = serialized_texts
end
2199/3/app/comment_analyzer.rb
Outdated
| end | ||
| end | ||
|
|
||
| def serialized_texts |
There was a problem hiding this comment.
If it's serialized_texts JSON, call it serialized_texts_json :)
2199/3/app/comment_analyzer.rb
Outdated
| end | ||
| # rubocop:enable Metrics/AbcSize | ||
|
|
||
| def endpoint |
There was a problem hiding this comment.
Again, what endpoint? I see from the code below that it's the #azure_endpoint. Why not say it explicitly?
| browser.visit(@link) | ||
| rescue StandardError => exception | ||
| attempts += 1 | ||
| sleep(2 * attempts) |
There was a problem hiding this comment.
Why did you choose 2*attempts formula?
There was a problem hiding this comment.
I use exponential backoff. I did something wrong??
| link = params[:link] | ||
| @page = Page.find(link: link).first | ||
| unless @page | ||
| comment_texts = OnlinerPageParser.new(link).top_comment_texts |
There was a problem hiding this comment.
I guess the Parser is redunant word here.
It's just OnlinerPage.new(link). And then you ask your OnlinerPage about its '#top_comments' and so on.
Or if you want to stick to comments, call it OnlinerPageCommentsParser
| @page = Page.find(link: link).first | ||
| unless @page | ||
| comment_texts = OnlinerPageParser.new(link).top_comment_texts | ||
| comments_with_score = CommentAnalyzer.new(comment_texts).analyze |
There was a problem hiding this comment.
Eventually, it's a Comment or Comments analyzer? From your code, I see that it analyzes all comments, not the particular one.
| erb :show | ||
| end | ||
|
|
||
| post '/pages' do |
There was a problem hiding this comment.
Let's meditate on this endpoint.
I'd like to have less code in here.
Something like this:
#app.rb
post '/pages' do
if Page.exists?(link: link)
@page = Page.find(link: link)
else
@page = Page.create_from(link: link)
end
erb :show
endIt's just an idea, but it will make your post action more verbose and will hide all Page and Comment creation logic.
Номер
2199
Номер задания
3
Ссылка на видео с демо
https://youtu.be/lQrc5G4Vhd8
Комментарии
The processing is so long, because the internet is not very good and Onliner is loaded for a long time