2396 - 3 #279
2396 - 3 #279
Conversation
|
https://youtu.be/aHM58_v8gng?t=1m24s Откуда там про Марс вообще :))) |
|
Опять-таки, не делай всю работу в одном коммите — лучше раздробить на много мелких, так проще откатывать и изменять код. |
| post.delete if post.link == params[:link] | ||
| end | ||
| PostWorker.perform_async(params[:link]) | ||
| sleep 2 |
There was a problem hiding this comment.
Я так понимаю, ты ждешь что твой парсер обработает страницу?
А что, если за 2 секунды PostWorker не отработает?
Если ты прям хочешь ждать до конца, то ты можешь вызывать PostWorker.perform(params[:link]).
Но вообще так тоже делать не круто (а делать через pub/sub), но наверное пока сойдет и так.
There was a problem hiding this comment.
Изначально хотел добавить flash как в рельсах, что ссылка отправлена на анализ, но не успевал и сделал такой костыль.
| <header> | ||
| <%= erb :navbar %> | ||
| </header> | ||
| <div class="container"> |
| @@ -0,0 +1,15 @@ | |||
| <%= erb :header %> | |||
| </div> | ||
| </div> | ||
| <!-- jQuery (necessary for Bootstrap's JavaScript plugins) --> | ||
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script> |
There was a problem hiding this comment.
JS и CSS скрипты мы обычно складываем в head.
|
|
||
| post '/create' do | ||
| Post.all.each do |post| | ||
| post.delete if post.link == params[:link] |
There was a problem hiding this comment.
Можешь рассказать зачем ты это делаешь?
Потому что на первый взгляд это кажется совсем не верным решением.
There was a problem hiding this comment.
Удаляю пост, если он уже был ранее создан. Обновить его данные было бы логичнее...
| end | ||
|
|
||
| get '/:id' do | ||
| @posts = Post.all |
| # This is class handling comments for post across API onliner.by | ||
| class CommentsParser | ||
| LIMIT = 50 | ||
| API_PATH = 'https://comments.api.onliner.by/news/tech.post/'.freeze |
There was a problem hiding this comment.
Я так понимаю из url, ты только технические статьи можешь парсить?
There was a problem hiding this comment.
Хз отчего, но я бы мб вынес эти констаны в Setting.onliner_api и сделал хешом. Подумай об этом. Делать необязательно, но мб тебе понравится.
There was a problem hiding this comment.
Согласен. Добавило бы больше гибкости
| # This is class conects to API AZURE, Analyze sentiment texts and | ||
| # fetch score for his | ||
| class RatingCounter | ||
| KEY_AZURE = Setting.get('key_azure').freeze |
There was a problem hiding this comment.
Вот это тоже бы почему-то вынес в Setting'и.
There was a problem hiding this comment.
Или даже может сделал AzureSetting < Setting и так инкапсулировал конфиг Azure. То же самое с онлайнером.
| class PostWorker | ||
| include Sidekiq::Worker | ||
| def perform(link) | ||
| call(link) |
There was a problem hiding this comment.
reek ругался, что лучше вынести в отдельный класс.
| end | ||
|
|
||
| def self.fetch_title(url) | ||
| agent = Mechanize.new |
There was a problem hiding this comment.
Мне не нравится то, что класс называется CommentsParser, а ты делаешь две разные вещи (фетчишь комменты и фетчишь тайтл).
По ООП тут правильно было бы вынести этот метод в отдельный класс. Либо (чтобы соптимизировать на запросе) фетчить заголовок вместе с комментами и возвращать хешом — {title: '...', comments: [...]}
There was a problem hiding this comment.
Второй вариант рассматривал, но посчитал, что лучше вынести в отдельный метод.
Вынесу в отдельный класс.
| end | ||
|
|
||
| def self.fetch_title(url) | ||
| agent = Mechanize.new |
There was a problem hiding this comment.
Второй вариант рассматривал, но посчитал, что лучше вынести в отдельный метод.
Вынесу в отдельный класс.
| class PostWorker | ||
| include Sidekiq::Worker | ||
| def perform(link) | ||
| call(link) |
There was a problem hiding this comment.
reek ругался, что лучше вынести в отдельный класс.
Номер
2396
Номер задания
3
Ссылка на видео с демо
https://youtu.be/aHM58_v8gng
Комментарии
Готово