Pull to refresh

Comments 18

А что мешает в контроллере сделать вот так:
class SomeController
  attr_writer :external_service

  def index
    authorize!
    external_service.accounts
  end

  def external_service
    @external_service ||= ExternalService.build
  end
end
Хороший подход, вполне позволяет писать тесты на контроллер. Но есть и минусы: когда в публичном интерфейсе класса есть attr_writer, можно решить, что где-то кто-то собирается поменять значение поля (или, по крайней мере, может это сделать). Особенно эти проблемы проявляют себя тогда, когда код сервиса начинает расширяться с помощью плагинов, хранящихся в отдельных репозиториях (например, при использовании модификаций одного и того же сервиса для разных компаний-заказчиков).
В общем, писать код с оглядкой на тесты — это одно. Писать код, который будет использоваться только в тестах, уже другое.

Мешает, например, то, что рельсы открывают файлы в одном им ведомом порядке, и рано или поздно придется или вручную указать порядок открытия (что плохо), или помнить и тщательно следить, чтобы вызов external_service=() случился раньше, чем вызов external_service(), что вообще ни в какие ворота.


На #||= нужно полагаться с очень большой осторожностью, тут Пиотр очень прав. Лучше его вообще избегать, в случае ожидаемого постороннего вызова external_service=(). Все эти рельсовые присвоения в before_*** — ровно отсюда выросли.

Недостаток такого подхода в том, что он не очень хорошо работает, когда в контроллере более одного экшена. Код становится сложно понимать.

Я очень люблю читать полемику Пиотра и Давида: оба настолько яростные евангелисты своего подхода, что за щепками, летящими от деревьев, там уже леса не видно вовсе.


Эта реализация Hacker ужасна. Хотя бы потому, что build принимает строку, а new — экземпляр Keyboard. В языке, поддерживающем pattern matching из коробки такой проблемы бы не было, но и в руби ее решить довольно просто:


class Hacker
  def self.build(keyboard_or_layout = 'us')
    new(keyboard_or_layout)
  end

  private_class_method :new

  def initialize(keyboard_or_layout)
    @keyboard = Keyboard === keyboard_or_layout ? keyboard_or_layout : Keyboard.new(layout: keyboard_or_layout)
  end
end

Если уж мы определили фабрику (тут-то зачем?! — ну ладно, типа, для примера) — ну и давайте тогда, пожалуйста, пользоваться фабрикой. Кому очень надо, вызовет Hacker.send(:new), тут же, напоминаю, руби.


Вообще, ничего плохого в открытых классах и monkey-patching’е нет, тут я целиком на стороне DHH. Кому не нравится — есть масса языков для перехода. Но код на руби должен выглядеть, как код на руби, квакать, как код на руби, и быть руби-кодом. А не php, записанным с использованием руби-синтаксиса. Все эти аргументы, мол, ой, ведь конфликты же — это вообще какой-то признак начинающейся шизофрении. Кто все эти люди, которые боятся, что 5.days.ago будет с чем-нибудь конфликтовать? С чем, стесняюсь спросить? С гемом, который переопределит Integer#days метод, чтобы он возвращал промилли? Так с этим гемом тогда наверняка есть еще 100500 гораздо более серьезных проблем.


Что же до аргументов Пиотра (точка зрения которого мне на самом деле ближе, я даже контрибьютор dry-transation), то DI — это очень хорошо, но не в приведенном примере. Потому что тест он на то и тест, чтобы mock. А mock в rspec — это monkey-patching. Оп-па.

Ну скажем, много лет назад натыкались, что mongoid что-то перекрыл в Object и nil? возвращал true на объекте (не NilClass).

Сейчас есть проблемка с гемом packable (да, я понимаю, что он не очень популярен, но все же факт есть факт).
Отдельно отлично работает.
В рельсовом проекте с большим количеством гемов, при некоторых условиях, есть исключения (причем сам packable, там казалось бы, ни к чему, но поскольку манкипатч… ну вы поняли). Причем выкурить, с чем именно конфликтует не удалось, т.к. усилий нужно неоправданно много, запихнул в group :manual_load в Gemfile и подключаю вручную там, где нужен. Пока работает.

Я отдаю себе отчет в этом:

> А mock в rspec — это monkey-patching. Оп-па.

И лично считаю rspec лучшим тулом для TDD. Хотя я скорее явист, чем рубист.
Минусы истекают из плюсов и наоборот.

ИМХО манкипатч 5.days.ago (в платформе!) это одно дело, и куда бы ни шло, а вот когда мелкие гемы лезут со своей ерундой в стандартные классы — совсем другое. Но тут уж ничего не поделаешь.
mongoid что-то перекрыл в Object и nil? возвращал true на объекте (не NilClass)
с большим количеством гемов, при некоторых условиях, есть исключения

Есть, конечно, исключения. Я сам умею в продакшене GI в корку уложить (точнее, сумел разок). Но это же все из серии «давайте запретим ножи, они острые».


когда мелкие гемы лезут со своей ерундой в стандартные классы

Смотря как лезут. Вот пример прямо из печки: https://github.com/am-kantox/iteraptor — добавляет пару методов для рекурсивной/глубокой итерации, маппинга и редьюса прямо в Hash и Array. В исходниках можно увидеть атавизм: реверанс в сторону «ни-ни-ни, не трожь базовые классы». А именно, сначала гем требовал эксплицитного require 'greedy', чтобы открыть стандартные классы. Уже в версии 2.0 — это дефолтное поведение. Потому что так удобнее, и потому что названия функций являются точными аналогами стандартных, только именованы на испанском языке.

>> Но это же все из серии «давайте запретим ножи, они острые».

Разумеется, просто неправильно выбранный инструмент.
Проблема манки-патчинга не в том, что после его использования у вас сразу выпадут волосы и зубы, а в том, что манки-патчинг по своей сути это слабо управляемый инструмент. Его не стоит сравнивать с ножом, скорее с топором, которым вы пытаетесь разрезать торт. Цели своей вы добъетесь, но выглядеть это будет диковато, даже если топор наточен очень остро. Но допустим, что руби это такой странный мир людей, предпочитающих такие методы решения проблем.
Манки-патчинг это большая головная боль для разработчиков гемов, потому что получается, что кто угодно может непредсказуемо поменять рантайм языка, в итоге выйдет, что какие-то гемы окажутся несовместимыми. Это вовсе не мифы, чем дольше руби развивается, тем больше таких проблем происходит. Вот пример с переполнением стека из-за переопределения базовых классов: https://github.com/txus/kleisli/pull/17. Хочу заметить, что ошибки с переполнением стека очень неудобно отлаживать, буквально на днях такая ошибка уронила у меня интерпретатор с ошибкой адресации, никакого руби-стека я не увидел вообще.
Дальше — больше. Есть рельсы, которые построены на activesupport. Если какое-то из поведений в AS ломает ваш гем, то вам ничего не остается, кроме как поменять вашу библиотеку для совместимости с AS, потому что иначе никто не будет ей пользоваться. Это уже не ножи, это пики из задачи двух стульев.
Стоит присмотреться повнимательнее какую же проблему на самом деле решает манки-патчинг. На деле это оказывается банальное отсутствие нужных абстракций или инструментов, вместо создания которых, люди патчат рантайм. Точка зрения dry-rb-коммьюнити заключается в том, что так быть не должно. Нужно разрабатывать библиотеки и подходы, которые будут удобны и функциональны без необходимости изменять поведение стандартных классов, для этого в руби есть все необходимое. Мы неоднократно видели, как продуманный подход преподносил много приятных сюрпризов впоследствии, что еще больше усилило нашу уверенность в правильности выбранного пути. Простите, если это звучит слишком пафосно :)
Я не знаю, что вы понимаете под “dry-rb community”, на всякий случай повторю: мой лично код есть в dry-transaction. Так что не надо мне про продуманные подходы, я в курсе. Но это не значит, что я буду слепо повторять за Пиотром все, что он в запале скажет. Кроме того, если вы приглядитесь к коду dry-rb, вы внезапно увидите всю мощь наплевательского отношения на общепринятые стандарты (например, прямое наследование от BasicObject, которое ломает дебаггер и прочие смешные вещи, которые явно выросли из мощи языковых средств и неумения/нежелания сделать по уму).

Инструмент для «безопасного» манкипатчинга в руби есть аж с 2.0, refinements называется, и никто им не пользуется. Потому что не удобно, и ничем не отличается от создания инстансов через `Class.new(BaseClass) do |c| c.prepend Patch… end`.

> На деле это оказывается банальное отсутствие нужных абстракций или инструментов,
> вместо создания которых, люди патчат рантайм.

Ерунда. Открытые классы — мощнейший инструмент, который позволяет сделать очень много, если делать это по уму. Если же делать что-нибудь не по уму, то даже в насквозь казалось бы безопасном Delphi в 1994 году мне удалось наступить на грабли с левой библиотекой, которая обращалась к приватным полям чужих объектов по смещению. В любом языке можно отравить жизнь всем своим пользователям, и проблема не в манкипатчинге.

Не сто́ит повторять за мэтрами все, что они скажут. Истина посередине.
Странно, что вы отказываете мне в наличии собственного мнения. В любом случае, я имел в первую очередь позицию core team.

BasicObject создан для использования в качестве контекста для DSL, так он и используется, разве нет? Ну, конечно, если что-то не так с этим подходом, и вы знаете об этом, нет смысла держать в себе, откройте тикет, где это можно будет обсудить в «рабочем порядке» :) Равно как и про «остальные смешные вещи».

refinements не взлетели потому что они на самом деле не решают основную проблему — отсутствие абстракций. Ну, я не исключаю, что если бы они работали как-то по-другому, то это позволило бы хотя бы снять вопрос об изменении рантайма.

Ерунда. Открытые классы — мощнейший инструмент, который позволяет сделать очень много, если делать это по уму. Если же делать что-нибудь не по уму, то даже в насквозь казалось бы безопасном Delphi в 1994 году мне удалось наступить на грабли с левой библиотекой, которая обращалась к приватным полям чужих объектов по смещению. В любом языке можно отравить жизнь всем своим пользователям, и проблема не в манкипатчинге.

Это пример ложной дилеммы.
> Странно, что вы отказываете мне в наличии собственного мнения.

Это где это? Если бы я отказывал вам в наличии собственного мнения, мне было бы неинтересно что-нибудь писать в ответ, по-моему это достаточно очевидно.

> BasicObject создан для использования в качестве контекста для DSL, так он и используется, разве нет?

Кхм. Ну, допустим. Хотя избавляться от неймспесинга и внедренных зависимостей можно и поизящнее, не разламывая в щепы pry, которая не ожидает от объектов такой подлости, как отсутствие `puts`. Делегировать пару методов из `Kernel` уж точно можно было бы. Впрочем, я в целом не понимаю, в чем прелесть `BasicObject` именно там, кроме демонстрации «идеологии».
Я не открываю тикеты потому, что не хочу лаять на Пиотра так, как он лает на DHH. Мне [местами] нравится подход dry, [местами] нравится подход rails и я хочу оставить их авторам полную свободу самовыражения. Они прекрасно смотрятся в паре.
Я не прошу/заставляю женщин сделать пластическую операцию ради меня, я выбираю тех, которые нравятся от входа.

> Это пример ложной дилеммы.

Нет, это вам кажется. Никакой дилеммы тут нет вообще. Мы обсуждаем манкипатчинг. Вы приводите примеры, в которых манкипатчинг нагадил. Я утверждаю, что нагадить в нашей специальности — не проблема вообще.

На совсем тривиальном примере: Петя отравился гнилым яблоком. Петя утверждает, что яблоки есть вредно. Я утверждаю, что отравиться можно чем угодно, но это не является причиной для отказа от собственно яблок (точнее, ниоткуда не следует, что отказ от яблок приведет к понижению вероятности последующих отравлений).
Кхм. Ну, допустим. Хотя избавляться от неймспесинга и внедренных зависимостей можно и поизящнее, не разламывая в щепы pry, которая не ожидает от объектов такой подлости, как отсутствие `puts`. Делегировать пару методов из `Kernel` уж точно можно было бы. Впрочем, я в целом не понимаю, в чем прелесть `BasicObject` именно там, кроме демонстрации «идеологии».

Я все же настаиваю на обсуждении этого на гитхабе, там более подходящее место. Но насчет пробрасывания методов из Kernel идея хорошая.

Что касается остального обсуждения, то я написал тут не с целью переубедить кого-то, а чтобы обозначить позицию. Манкипатчинг по нашим представлениям крайне редко решает проблему адекватно задаче, при этом создает значительные неудобства, потому что люди не могут предвидеть последствия изменения стандартных классов, это слишком сложная задача для человека. Поэтому мы пользуемся другими инструментами для решения задач.
Дык а я обозначаю свою позицию: вы ошибаетесь :)

Это не значит, что кто-то прав, кстати. Я прекрасно понимаю, в чем сухие монады чище кляйсли. Я ни в коем случае не призываю открывать `NilClass` каждый раз, когда нам лень проверить операнд (я вообще считаю `try` наибольшим злом с точки зрения воздействия на неокрепшие умы, принесенным рельсами).

В то же самое время, как мне кажется, `5.days` не может ничего сломать, нигде. Нас приглашают поучаствовать в разработке ruby core, через открытые классы — зачем отказываться? Матц напрямую неоднократно говорил, что да, вот такая цель. Да, можно создать гем, который всем все сломает — так есть 100500 гораздо более простых способов все сломать. Не нужно тянуть в рот всякую гадость, не нужно устанавливать `leftpad`, но все хорошо в меру.

Я убежден, что ваша команда пишет внятный работоспособный код не потому, что вы решили отказаться от манкипатчинга.
5.days может сломать, я разбирался с такой проблемой, когда Duration притворялся числом, его нельзя было распознать никак, кроме как вызвать метод parts, который внезапно оказывался/не оказывался у объекта, так можно было отличить целое число от Duration. Это было в рельсах 3.2, с тех пор этот момент улучшили, но я лично потратил час или два на разбирательства с той проблемой, какой-нибудь новичок, по-моему просто погиб бы. Но это я в качестве примера, на самом деле я согласен, что патчинг бывает разный, но беда в том, что вместе в рельсами вы получаете все и сразу, нет способа отказаться. А когда вы на это жалуетесь, вам рекомендуют использовать другой язык программирования под предлогом того, что вы не умеете в руби, такие дела.
> беда в том, что вместе в рельсами вы получаете все и сразу, нет способа отказаться

> когда вы на это жалуетесь, вам рекомендуют использовать другой язык программирования
> под предлогом того, что вы не умеете в руби

Вот не нужно передергивать, пожалуйста. Когда вы жалуетесь (вполне обоснованно) на рельсы — вам рекомендуют не использовать рельсы.

А вот когда вы называете манкипатчинг глобальной проблемой руби — тут вам рекомендуют сменить язык, потому что в руби открытые классы, манкипатчинг, неприватные приватные методы, метапрограммирование — ядро, ради которого все задумано (насколько я понимаю). Идея такая: чтобы хороший разработчик не отстрелил себе ногу, нужно не пистолеты запрещать, а курсы стрельбы пропагандировать.

Не нужно проблемы, созданные увлекшимся игрой в «давайте запатчим все на свете» Давидом, переносить на руби. Не нужно объявлять манкипатчинг — злейшим злом _языка_. Я ведь только об этом говорю.

И это, если вас не затруднит, а можно поподробнее (линк на неработающий код пойдет), как Duration, притворившийся числом, все поломал? Зачем мне знать, инстанс чего это у меня, если он квакает подходящим образом?
Да я не про вас, это аргумент dhh :)

Про duration я ничего уже не помню, но проблема при дебаге была в том, что квакание там было неподходящим, is_a? и .class тоже не работали, выдавая Fixnum, на самом деле им не являясь. Конечно, если там поразбираться, наверняка была проблема в некорректном обращении с типами в руби, но вот разбираться с такими вещами тяжеловато.
Sign up to leave a comment.

Articles