Conversation
|
@take-five @SemenMolokanov @vkuznetsov посмотрите |
lib/reactor.rb
Outdated
There was a problem hiding this comment.
ох 😔
нужно, чтобы все require-ы в проекте выглядели так:
require 'reactor/dispatcher'
|
Говоря кратко - я ни хуя не понял |
|
предлагаю делать документацию к классам |
lib/reactor/connection.rb
Outdated
|
+1 к документации В документации обязательно написать примеры использования. Вообще говоря, написание примеров должно сподвигнуть к рефакторингу. |
lib/file_connection.rb
Outdated
There was a problem hiding this comment.
FileConnection - соединение файла?
Здесь и имя класса неподходящее и почему то, что делает работу с файлами наследуется от коннекта?
Надо отделить от сервера слой, в котором выполняется конкретная работа.
|
upd |
lib/reactor.rb
Outdated
There was a problem hiding this comment.
Вот опять повсюду это...
Я же писал, что нужно, чтобы require-ы выглядели так: require 'reactor/dispatcher'
|
Я вижу over-engineering. Много абстракций непонятного назначения, принцип работы кода неясен, у переменных малоописательные названия. Попробуй в проектировании идти от интерфейса:
Именно тесты помогут создать хороший, понятный дизайн. |
|
основывался на статье Шмидта 1995 года |
lib/reactor/data_handler.rb
Outdated
There was a problem hiding this comment.
Почему DataHandler знает что-то про epoll?
There was a problem hiding this comment.
это косяк, знаю )
пытался поначалу сделать события Dispatcher'а
Reactor::ACCEPT_EVENT = 1
Reactor::READ_EVENT = 2
Reactor::WRITE_EVENT = 4
...
запутался и не реализовал
|
Следует привести дизайн в соответствие с принципами [SOLID](http://en.wikipedia.org/wiki/SOLID_(object-oriented_design\)) |
lib/reactor.rb
Outdated
There was a problem hiding this comment.
unshift для путей загрузки и будет счастье
|
совсем забыл - UPD же |
There was a problem hiding this comment.
Вместо самостоятельного написания можно было воспользоваться классом Struct:
Manager = Struct.new(:dispatcher, :data_handler_class, :initializer)Но не в этом дело. А дело в том, что это класс, не выполнящий вообще никакой полезной работы. Это уже код с душком (см. М.Фаулер, "Рефакторинг"). Такой запах называется Middle Man.
There was a problem hiding this comment.
Т.е., этот класс занимается исключительно делегированием, вносит дополнительную косвенность, увеличивает сложность и так непростого кода, и сильно затрудняет понимание вообще всего
There was a problem hiding this comment.
И вдобавок имеет невыразительное название 8)
There was a problem hiding this comment.
Ну это же совершенно не ясно из кода
|
У тебя сейчас получилась сложная архитектура с кучей делегирования, хотя, как мне кажется, можно использовать особенности ruby (а именно блоки), и тогда реактор может выглядеть как-то так (прошу заметить, это не дизайн-проект реактора, а лишь набросок, эскиз): reactor = Reactor::Dispatcher.new
reactor.read(socket) { |data| puts data }
reactor.write(socket) { |data| socket.write(data) }
reactor.error(socket) { whatever }
reactor.runОбрати внимание, что при таком подходе не нужны гроздья классов, в которые делегаторы будут делегировать делегирование полномочий. |
reactor = Reactor::Dispatcher.new
reactor.read(socket) { |data| puts data }
reactor.write(socket) { |data| socket.write(data) }
reactor.error(socket) { whatever }
reactor.runв этой схеме не учитывается прием подключений, надо подумать кто их обрабатывать будет |
|
тут теперь конфликты надо порешать ))) |
|
ага, ждите коммит. амазон поседеет от страха перед нашим продуктом, ну или от старости ) |
5 participants
Передача подключения воркерам