Читаю я значит код,

а он мне как раз, чтобы пригореть и написать в бложик. Потому что это, блядь, пиздец, логика размазана по моделям ровным слоем, скопирована / вставлена куда попало, а новый мейнтейнер при слове «абстракция» медленно седеет.

Хотя и в отрыве от безумств абстракции там есть файл на 500+ LOC, который видимо пытались хоть чуть чуть подрефачить.

Так как общее правило «пригорел - отписал в бложик» все еще действует, напишу свое очень важное мнение по поводу того, как правильно делать классы и как правильно рефакторить

Корень проблемы

На мой взгляд, философия ООП в целом недостаточно разборчива к семантике наследования, причем настолько, что SOLID это не просто памятка программисту, а целая методика, которую приходиится учить наизусть людям, вполне освоившимися в мире ООП.

Вот классический пример из книжки

class Animal
  def voice
    raise NotImplementedError
  end

  def voice!
    puts voice
  end
end

class Cat < Animal
  def voice
    "meow"
  end
end

class Dog < Animal
  def voice
    "wow"
  end
end

class Doge < Dog
  def voice
    "such wow"
  end
end

Он простой, понятный, и на самом деле абсолютно бредовый. Да, raise NotImplementedError добавлен для острастки, по хорошему нужно было писать как то так

class Animal
  # @!method voice
  #   @return [String] animal voice

  def voice!
    puts voice
  end
end

или как то еще в зависимости от того, какой системы документации вы придерживаетесь, но это все руби-специфично, так что насрать.

Задачка-минутка: что не так с этим кодом с точки зрения SOLID?

Single responsibility principle

Этому походу вообще никого не учат. Хотя, стоит отметить, что это принцип, а не правило, а правило в общем случае KISS

То есть, если мы увидим онтологию именно в таком виде, оно по началу даже к лучшему, но не дай бог вы решите научить животное не только команде “голос”

Паттерн интерфейс, к сожалению, нормально сделан только в Haskell (и возможно в других языках семейства ML), поскольку реализация интерфейса находится в теле самого класса. Поэтому интерфейсы по хорошему следует реализовывать в паттерне “декоратор”. Выглядит это как то так

assets/2017-12-14-inteface/interface-via-decorator.dot

Это тот самый момент, когда на помощь приходит принцип KISS. Без завязания в абстракциях, чистый код написать невозможно, но если ваш интерфейс достаточно сложный (например, включает в себя много методов), то декоратор это круто. Не самой плохой идеей будет класть маленькие декораторы в тот же файл, что и декорируемый объект, но тут важно следить за количеством LOC в файле

The Open Closed Principle

Одна из самых больных тем в SOLID. В двух словах он звучит как «делайте классы расишряемыми при наследовании». Прицнип очень древний, поэтому лучше всего он выглядит в… C++. Редчайших случай, кстати.

Смысл в том, что чтобы добавить к классу функционал, его не надо переписывать, а надо от него наследоваться, и это САМОЕ ЕБУЧЕЕ БЛЯДЬ ГОВНО, КОТОРОЕ Я КОГДА-ЛИБО СЛЫШАЛ

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

Это все равно часто приводит к размазыванию кода. Перед тем, как думать о выносе какой-либо общей логики в класс-родитель, подумайте о выносе этой логики в сервисные классы, построенные в полном соответствии с Single responsibility principle.

Хорошей идеей этого принципа является отказ от использования super в пользу абстрактных методов. Увидел super в методе из 20 LOC — посмотрел git blame — уебал

# Bad
class Foo
  def foo
    # 30 LOC
  end
end

class Bar < Foo
  def foo
    # 30 LOC
    super
  end
end

# Good

class Foo
  def foo
    bar
    # 30 LOC
  end

  #!@method bar
end

class Bar < Foo
  def bar
    # 30 LOC
  end
end

Разумеется, если семантика bar что то вроде before_foo, уебать надо тоже. Возможно даже дважды

Если вам реально необходимо что то вроде before_foo, совершенно разнящееся между классами, попробуйте анти-паттерн Callback, станет чуть проще жить. Разумеется, лучше так не делать, но чего не сделаешь ради KISS

Иногда колбэки являются частью хорошего, годного паттерна (например, валидатор). В этом случае правильное решение состоит в том, чтобы декларативно перечислить то, что должно быть сделано перед foo. Но если кто то найдет там зависимость между колбэками в разных файлах, уебать все-таки надо. Или хотя бы декларативно опишите ее (скрытая реклама Verifly)

Liskov Substitution Principle

Тесты на базовый класс должны проходить, если его заменить наследником.

Поэтому не пишите тесты на базовый класс

Наследуясь от не-абстрактного класса, не смейте менять семантику его методов.

Опять же, C++ при всей его топорности впереди планеты всей, если метод не объявлен как виртуальный, то нехер его переопределять, если объявлен, то это имплементация по умолчанию.

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

Существует классический антипаттерн “наследование вбок”, который порожден сумеречным гением The Open Closed Principle в интерпритации Мейера. Это прицип 1988 года, который за следующие 10 лет был преосмыслен.

Наследование вбок это популярная тема, когда логика абстрактного класса пишется в одном из наследников и полностью ломает семантику наследования.

Interface Segregation Principle

Это Single responsibility principle для интерфейсов. Для чего повторять дважды? А потому что с первого раза не поняли потому что ответственность интерфейса не соответсвует ответственности модели. Например, если у вас есть модель ORM, ее интерфейсы явно имеют иное предназначение. Например, интерфейс формы для ее редактирования и декоратор для отображения это все-таки разные сущности.

Раз уж это правило про лишний раз напомнить, напомню, что интерфейсы правильно делать используя декораторы.

Dependency inversion principle

Используйте абстракции

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

Серьезно, не пытайтесь использовать этот прицип, если без него код выглядит нормально.

Индикатором того, что пора, является копи-паст кода. Вот в этот самый момент запилите абстракцию, чтобы его не копипастить. Вроде просто, но в реальности это сложный и важный принцип, который я объяснить не могу, да и не факт, что сам до конца понимаю.

И пожалуйста, не пишите логику одной фичи в двух местах сразу, если можно написать ее в третьем и только в третьем. Но без фанатизма

tl;dr

  • Наследуйтесь только от абстрактных классов или имплементаций по умолчанию.
  • include это тоже наследование
  • Не наследуйтесь вбок!
  • Если вам нужно больше одного метода, чтобы реализовать фичу, сделайте отдельный декоратор
  • Переписывайте, блядь, классы, наследование решает только проблемы интерфейса
  • Если вам нужно отрефачить жирный класс, выносите публичные методы целиком, а не пытайтесь разбить их на маленькие кусочки. Публичный метод это контракт, а не хуй собачий
  • Не пишите super, если этого можно избежать используя абстрактный метод
  • Даже колбэки лучше, чем super
  • Имплементацию по умолчанию легко спутать с имплементацией, комментируйте код, если средств выразительности языка (например, virtual) не хватает
  • Не копипастите код, а катпастите в абстракцию
  • И еще раз, НЕ ПИШИТЕ, БЛЯДЬ, КОД ОДНОГО МЕТОДА В НЕСКОЛЬКИХ ФАЙЛАХ. ВОНЯЕТ ПИЗДЕЦ
  • KISS