Skip to content

Converter#11

Open
AlexeyRyabchikov wants to merge 13 commits intoulstu:masterfrom
AlexeyRyabchikov:converter
Open

Converter#11
AlexeyRyabchikov wants to merge 13 commits intoulstu:masterfrom
AlexeyRyabchikov:converter

Conversation

@AlexeyRyabchikov
Copy link
Copy Markdown

No description provided.

bin/converter.rb Outdated
require 'open-uri'
require 'nokogiri'
require 'json'
require '../lib/readers/link_reader'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

А через require_all не пробовал добавлять всю папку сразу?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

отладил, заработало.

bin/converter.rb Outdated
require 'open-uri'
require 'nokogiri'
require 'json'
require '../lib/readers/link_reader'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Вообще, подключать файла из папки lib надо в main. В этом файле надо подключать библиотеку optparse и сам main файл. Bin/converter мы можем убрать и должны мочь запустить только main, например, из консоли, и он должен заработать.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Заработало )

result_xml += "</entry>\n"
end
result_xml += "</feed>"
File.open("../test/fixtures/file1", "w") do |file|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Добавь расширение к файлу, например, data.atom.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Добавил.

@@ -0,0 +1,14 @@
class Converter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Предпочтительнее переименовать в BaseConverter

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Переименовал.

file_contents.each do |line|
data += line
end
return data
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return можно не писать

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Убрал.

def self.read(input)
if input.include?('http://')
LinkReader.read(input)
else FileReader.read(input)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Проблема с отступами и переносами.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Исправил при помощи  rubocop -a



class Reader
attr_accessor :input
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Можно не объявлять. А в методе read обратиться, как @input

file_contents.each do |line|
data += line
end
return data
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return не нужен

require 'open-uri'


class Reader
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

А зачем объявлять данный класс? Надо использовать тот, который в программе main используется, мы его тестировать должны.

@@ -0,0 +1,19 @@
class Main
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PARSERS = ['JsonParser', 'AtomParser', 'RssParser']

main.rb Outdated

def run
data = Reader.read(@input)
parsed_data = Parser.parse(data)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Main::PARSERS.each do |parser_name|
return parser_name if Module.const_get(parser_name).can_parse?(data)
end

}
end
result
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

def self.can_parse?(data)
data.include?('{')
end

result_xml += "</item>\n"
end
puts result_xml += '</rss>'
File.open('../test/fixtures/file1.rss', 'w') do |file|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Я бы всё-таки разбил функционал метод на 2 - непосредственно конвертацию в xml и записб в файл.

main.rb Outdated
parsed_data = Module.const_get(input_format).parse(data)
if @sort == 'asc' || @sort == 'desc'
sorted_data = @sort == 'asc' ? AscSorter.sort(parsed_data) : DescSorter.sort(parsed_data)
convert_data = BaseConverter.new(@output)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Дублирование кода! И ниже ещё 2 строчки. Надо убрать.

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