Skip to content
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

add_client_file_and_gem_nokogiri #2

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

Conversation

gambro
Copy link

@gambro gambro commented Jul 10, 2017

No description provided.

Copy link
Contributor

@vickodin vickodin left a comment

Choose a reason for hiding this comment

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

Напиши пожалуйста, пример использования этого гема:
Что мне набрать в консоли, чтобы посмотреть, как он работает?

end

def get
if page
Copy link
Contributor

Choose a reason for hiding this comment

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

#get

Мы договорились, что:

get возвращает true или false в зависимости от результата (см. #success?)

end

def code
open(url).status.first
Copy link
Contributor

@vickodin vickodin Jul 10, 2017

Choose a reason for hiding this comment

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

Запрос урла должен быть совершен один раз. Up, а у тебя - здесь (#code) и в #page

Up: Отредактировал сообщение.

Copy link
Author

Choose a reason for hiding this comment

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

не придумал как по-другому. Добавив один метод на получение файла через open-uri избавился от дубля. Может можно лучше сделать, пока думаю

@gambro
Copy link
Author

gambro commented Jul 10, 2017

чтобы он заработал надо будет набрать в консоли такую конструкцию: FragmentCatcher::Client.new(url: url, css: css)
сделал в отдельном файле для возможности расширения базовой логики

@vickodin
Copy link
Contributor

FragmentCatcher::Client.new(url: url, css: css)

У нас по условию, никакого класса Client нет:
#1

fc = FragmentCatcher.new(url: 'url', css: 'css')

@gambro
Copy link
Author

gambro commented Jul 10, 2017

по поводу кода http. Другого способа кроме разбивки на метод не нашел. Nokogiri не предоставляет данный функционал. Можно было использовать mechanize для этого, там есть возможность сразу получить head и code

end

def body
page.at('body').inner_html
Copy link
Contributor

Choose a reason for hiding this comment

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

body возвращает не html body, а тело ответа https://en.wikipedia.org/wiki/HTTP_message_body
т.е. все равно что сделать в линукс консоли

GET http://127.0.0.1

@vickodin
Copy link
Contributor

по поводу кода http. Другого способа кроме разбивки на метод не нашел.

Не понял фразы. Можешь переформулировать? В чем сейчас проблема? А то я не понимаю про что речь. Спасибо!

get_fragments
end

def success?
Copy link
Contributor

Choose a reason for hiding this comment

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

Этот метод можно заменить одной строчкой:

def success?
  fragments.any?
end

@css = css
end

def get
Copy link
Contributor

Choose a reason for hiding this comment

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

Проведи оптимизацию этого метода пожалуйста.

@vickodin
Copy link
Contributor

  • Добавь пожалуйста один тест, как мы обсудили.
    Спасибо!

@vickodin
Copy link
Contributor

А тесты проходят?

@gambro
Copy link
Author

gambro commented Jul 11, 2017

да

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

Successfully merging this pull request may close these issues.

2 participants