blinnov’s blog

Собрал тут свой собственный список признаков факапнутости кода.

На первом месте – классы со словом “Manager” в названии. Безоговорочный лидер в номинации “превратим любой код в страшные спагетти”. Самое ужасное в таких классах это то, что делаются они из самых лучших побуждений. Впрочем, так всегда – самые кошмарные деяния совершаются под прикрытием лозунга “мы же хотим тебе лучшего”. Я специально не пишу о внутренностях этих классов, само название “Manager” для меня уже достаточное основание к тому, чтобы код полетел в помойку.

Второе место достается небезопасным преобразованиям типов. Ну, это когда в коде появляется преобразование от класса-предка к классу-потомку. Вроде этого

class A;
class B: public A;

void DoSomething(A* ptr)
{
(B*)ptr->MethodOfB();
}

100% признак проявления идиотии в проектировании иерархии классов. Особый шик, когда подобный downcast встречается в одном из базовых классов – это уже индикатор того, что пора сушить весла. Исключений не бывает. Если же такой код появляется в процессе рефакторинга там, где ничего подобного до того не было, то сушить весла нужно отправлять аффтара этого рефакторинга.

Третье место – классы “универсальный всемогутор”.  Близкий родственник класса “Manager”, протащеный оным на ключевую позицию в проекте, способен отравлять жизнь двум-трем поколениям программистов, которым выпадет этот код поддерживать. Обычно характеризуются тучей публичных методов, никак не связанных по смыслу ни друг с другом, ни с названием и предназначением класса и не изменяющих и не использующих ни одного из свойств класса, но при этом зачастую имеющих тучу неявных связей друг с другом.

Четвертое место – преднамеренное нарушение энкапсуляции. Это когда в класс “бутылка пива” пытаются заодно вкрячить нечто, что вроде как должно научить экземпляры его автоматически упорядочиваться внутри класса “ящик пива”. Когда через неделю закономерно выяснится, что бутылки нужно упорядочивать в сикспаки, начинается любимый в народе аттракцион “аврал”. Есть вообще только один случай, когда объектам допустимо иметь какие-то знания о контейнере, в которые их будут складывать – это преднамеренно делается интрузивный контейнер. Во всех остальных случаях такой код – индикатор того, что code review можно прекращать прямо сейчас, иначе проблемы этот код начнет доставлять уже на следующей неделе. В общем случае – запихивание в классы внешних нерелевантных зависимостей есть большое зло.

Пятый признак – преднамеренное игнорирование или извращенное употребление паттернов программирования. Когда из-за страха перед великим и ужасным шаблоном “фабрика” образюутся иерархии, где один-единственный параметр конфигурации спускается каждым промежуточным классом вниз на десять уровней, чтобы там ему где-то делали if или switch..case, когда при программировании на C++ боятся использовать RAII или бьют по рукам за bind; или же, наоборот, пихают повсюду smart pointers (доморощенные, конечно же, бо boost – это слишком сложно), жди беды. Очень быстро кто-то посередине той иерархии забудет, зачем нужен этот мистический параметр, нигде в обозримых окрестностях не применяющийсяи “пофиксит” код, обеспечив остальных неделями отладки (ведь юнит-тесты для такого кода писать не принято – он же очень сложный!); неиспользование RAII рождает трудноуловимые дедлоки, а умные поинтеры дают вкусить циклических зависимостей по полной.

Шестой признак – использование наследования там, где нужна агрегация. Например, есть интерфейс классов, есть базовый класс, которые реализует 85% всей требуемой функциональности. Позднее выясняется, что функциональность реализована вообще на 100%, а единственное требуемое различие – это разное поведение в случае создания класса с определенными внешними параметрами. Например, в реальной жизни есть у нас автомобиль. Точнее, его кузов. Во всем мире все производители просто ставят разные двигатели на один и тот же кузов и обзывют это обидным словом “комплектация”. В мире программирования часто происходит закат разума за ум – вместо того, чтобы создать класс “автомобиль” со свойством “двигатель”, начинают городить иерархию “автомобиль А с двигателем Б”, автомобиль Б с двигателем А” и так далее, что вообще противоречит здравому смыслу. Еще бы, класс автомобиль оказывается наследником класса “двигатель”. Проблема тут в том, что разобраться во всем этом хитросплетении и потом поддерживать ее становится решительно невозможно. В то же время правильный вариант – создать нужный объект тпиа “двигатель” и передать его новому объекту типа “автомобиль” при инициализации- не только сэкономил бы пару тысяч строк кода, но и позволил бы унифицировать и протестировать все имеющиеся автомобили и двигатели отдельно друг от друга и даже попробовать впоследствии легко примерить движок от Мерседеса на Запорожец.