Разбить God class?

Question

[Alex Wells]

Здравствуйте. Написал пакет для парсинга документации, в результате всю работу делает один "God class". Некоторые методы просто огромны. Вот пример метода: тык

Трейты - не выход, так как нормальной поддержи автокомплита у них нет (только паблик свойства и методы). Тэг Laravel прикрепил потому что не знаю, есть ли в laravel какие-то штуки, которые помогут с этим.

Вопрос: какие есть практики, что прочитать/посмотреть, какой код перерыть что бы иметь понятие о том, как это сделать правильно? Правильно это так, что бы не стыдно показать и с возможностью покрыть юнит тестами (сейчас только фичер).

Заранее спасибо.

UPD: Вот еще прикрутил CI один, так RouteWrapper портит оценку с A на F (минимально возможную) исключительно из-за сложности класса: тык

Comments

[Alex Wells]

Andrew Stark, и? Это не означает, что это нормально. Такой код невозможно разбить на методы и нормально протестировать частями.

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

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

Спасибо за ответ, но по делу ничего не сказали(

Answer 1

[Сергей]

навскидку (не разбираясь с логикой класса)
у тебя много методов зависят просто от $this->route
их уже можно выделить в один класс.
если есть аналогичные моменты от других переменных поступать также.
Вообщем разбиение делается так: анализ структуры класса и выделение подсттруктур переменных + методов. Эти подструктуры будут внутренне сильно связаны. И слабо связаны между собой. Это будет намек, что можно выделять классы из текста.
Пониманию структуры может помочь анализ ответственности год-класса.
Как бы первое, что нужно сделать. Остальное станет ясным по мере разбиения.

Comments

[Alex Wells]

у тебя много методов зависят просто от $this->route

это его работа) Там не "много" методов от него зависят, а вообще все) Если уж что и выносить, но все кроме $this->route))

А за остальное - спасибо!

[Сергей]

Alex Wells, философский вопрос на счет "его работа")
класс нужно разбить. поэтому не всегда нужно зацикливаться на названии. и работе.(сразу)
у тебя получается W->R (wrapper зависит от route)
но на деле W->A->R wrapper зависит от некого adapter, который зависит от route
это уже можно использовать для разбиения.
Затем action:
getControllerReflection, getMethodReflection зависят от parseAction (тот в свою очередь от getAction). т.е. это намек на то, что это обособленная функциональность - на деле скрытый класс. Можно выделять.
Заметь я не анализировал как должны называться классы, что они должны делать, но смог найти уже некую структуру и возможность разбить большой класс на несколько меньших. Уже затем можно подумать как называть и какова ответственность этих классов.
Принцип ответственности это не всегда вопрос "что делает класс". Это вопрос имеет ли класс одну причину для изменения.