Слишком много абстракций, затрудняющих расширение кода

9

У меня проблемы с тем, что я считаю слишком большой абстракцией в базе кода (или, по крайней мере, имею дело с ней). Большинство методов в кодовой базе были абстрагированы для получения самого высокого родителя A в кодовой базе, но у дочернего элемента B этого родителя есть новый атрибут, который влияет на логику некоторых из этих методов. Проблема в том, что эти атрибуты не могут быть проверены в этих методах, потому что входные данные абстрагированы от A, и, конечно, A не имеет этого атрибута. Если я пытаюсь создать новый метод для другой обработки B, он вызывается для дублирования кода. Предложение моего технического лидера состоит в том, чтобы создать общий метод, который принимает логические параметры, но проблема в том, что некоторые люди рассматривают это как «скрытый поток управления», где общий метод имеет логику, которая может быть не очевидна для будущих разработчиков , а также этот совместно используемый метод будет чрезмерно усложняться / извиваться один раз, если будущие атрибуты необходимо будет добавить, даже если он будет разбит на более мелкие совместно используемые методы. Это также увеличивает связь, снижает сплоченность и нарушает принцип единой ответственности, на который указал кто-то из моей команды.

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

YamizGers
источник
10
добавление примера кода для неграмотной «проблемы» поможет лучше понять ситуацию
Seabizkit
Я думаю, что здесь нарушены два твердых принципа. Одиночная ответственность - если вы передадите логическое значение функции, которая должна управлять поведением, функция больше не будет нести одну ответственность. Другой - принцип подстановки Лискова. Представьте, что есть функция, которая принимает класс A в качестве параметра. Если вы перейдете к классу B вместо A, функциональность этой функции будет нарушена?
Бобек
Я подозреваю, что метод A довольно длинный и делает больше чем одно. Это тот случай?
Rad80

Ответы:

27

Если я пытаюсь создать новый метод для другой обработки B, он вызывается для дублирования кода.

Не все дубликаты кода созданы равными.

Допустим, у вас есть метод, который принимает два параметра и складывает их вместе total(). Скажем, у вас есть еще один называется add(). Их реализации выглядят полностью идентичными. Должны ли они быть объединены в один метод? НЕТ !!!

Принцип « Не повторяйся сам» или « СУХОЙ» - это не повторение кода. Речь идет о распространении решения, идеи, так что, если вы когда-нибудь измените свою идею, вам придется переписывать везде, где вы распространяете эту идею. Blegh. Это ужасно. Не делай этого. Вместо этого используйте DRY, чтобы помочь вам принимать решения в одном месте .

Принцип СУХОГО (не повторяй себя) гласит:

Каждая часть знаний должна иметь одно, однозначное, авторитетное представление в системе.

wiki.c2.com - Не повторяйся

Но DRY может быть испорчен в привычку сканировать код в поисках аналогичной реализации, которая кажется копией и вставкой где-то еще. Это мертвая форма мозга СУХОГО. Черт, вы можете сделать это с помощью инструмента статического анализа. Это не помогает, потому что игнорирует пункт СУХОЙ, который заключается в сохранении гибкости кода.

Если мои общие требования изменятся, мне, возможно, придется изменить мою totalреализацию. Это не значит, что мне нужно изменить мою addреализацию. Если какой-нибудь придурок свел их вместе в один метод, то теперь мне нужна небольшая ненужная боль.

Сколько боли? Конечно, я мог бы просто скопировать код и создать новый метод, когда мне это нужно. Так что нет ничего страшного, верно? Malarky! Если ничего другого, ты стоишь мне доброго имени! Хорошие имена трудно найти и плохо реагируют, когда вы играете с их значением. Хорошие имена, которые ясно дают понять намерения, важнее, чем риск того, что вы скопировали ошибку, которую, честно говоря, легче исправить, если ваш метод имеет правильное имя.

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

С другой стороны, если у вас есть sum()метод, который имеет идентичную или даже отличную от реализации реализацию total(), тем не менее, когда бы ни менялись ваши общие требования, вам придется менятьsum() тогда есть большая вероятность, что это одна и та же идея под двумя разными именами. Мало того, что код будет более гибким, если они будут объединены, он будет менее запутанным в использовании.

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

Теперь вы, кажется, перегорели на абстракции. Это очень плохо, потому что абстракция - замечательная вещь, когда все сделано хорошо. Вы используете это много, не думая об этом. Каждый раз, когда вы водите автомобиль, не разбираясь в системе реечной передачи, каждый раз, когда вы используете команду печати, не думая о прерываниях ОС, и каждый раз, когда вы чистите зубы, не думая о каждой отдельной щетине.

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

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

Это абстракция. Как клиент, я даже не знаю, с чем говорю. Я просто знаю, что мне нужно от этого. Если это означает, что вы должны завернуть что-то, чтобы изменить его интерфейс, прежде чем передать его мне нормально. Мне все равно Просто делай то, что мне нужно. Хватит усложнять.

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

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

Это отношение породило инъекцию зависимостей (или просто отсылку, если вы, как я, в старой школе). Это хорошо работает с предпочтительным составом и делегированием по наследованию . Отношение идет под многими именами. Мой любимый - скажи, не спрашивай .

Я мог бы утопить тебя в принципах весь день. И, похоже, твои коллеги уже есть. Но вот в чем дело: в отличие от других областей техники этому программному обеспечению менее 100 лет. Мы все еще выясняем это. Так что не позволяйте кому-то с большим количеством устрашающего звучания книжного обучения запугивать вас писать трудный для чтения код. Слушайте их, но настаивайте, чтобы они имели смысл. Не принимай ничего на веру. Люди, которые каким-то образом пишут код только потому, что им сказали, что это так, даже не зная, зачем создавать самые большие беспорядки из всех.

candied_orange
источник
Я искренне согласен. DRY - это трехбуквенная аббревиатура для фразы из трех слов «Не повторяйся», которая, в свою очередь, представляет собой статью на 14 страницах в вики . Если вы все это слепо бормотать эти три буквы , не читая и понимания 14 страничную статью, вы будете работать в неприятности. Он также тесно связан с « Один раз и только один раз» (ОАОО) и более слабо связан с « Единой точкой истины» (SPOT) / «Единственным источником истины» (SSOT) .
Йорг Миттаг,
«Их реализации выглядят полностью идентичными. Должны ли они быть объединены в один метод? НЕТ !!!» - Обратное также верно: то, что два фрагмента кода различны, не означает, что они не являются дубликатами. На вики-странице OAOO есть замечательная цитата Рона Джеффриса : «Однажды я видел, как Бек объявил два патча почти совершенно другого кода как« дублирующие », изменил их так, чтобы они БЫЛИ дублировали, а затем удалил вновь вставленное дублирование, чтобы появиться с чем-то явно лучше ".
Йорг Миттаг,
@ JörgWMittag конечно. Важнейшей вещью является идея. Если вы дублируете идею с другим кодом, вы все равно нарушаете.
candied_orange
Я должен представить, что статья на 14 страницах о том, как не повторяться, часто повторяется.
Чак Адамс
7

Обычное высказывание, которое мы все читаем здесь и там:

Все проблемы можно решить, добавив еще один уровень абстракции.

Ну, это не правда! Ваш пример показывает это. Поэтому я бы предложил слегка измененное утверждение (не стесняйтесь использовать повторно ;-)):

Любая проблема может быть решена с помощью ПРАВИЛЬНОГО уровня абстракции.

В вашем случае есть две разные проблемы:

  • чрезмерно обобщение , вызванное добавлением каждого метода на абстрактном уровне;
  • фрагментация из конкретного поведения , которые приводят к впечатлению не получить большую картину и чувства теряется. Немного как в цикле событий Windows.

Оба связаны:

  • если вы абстрагируете метод, в котором каждая специализация делает это по-разному, все в порядке. Никто не имеет проблемы с пониманием того, что он Shapeможет рассчитать его surface()специализированным способом.
  • Если вы абстрагируете какую-то операцию, где есть общий общий поведенческий паттерн, у вас есть два варианта:

    • либо вы будете повторять обычное поведение в каждой специализации: это очень избыточно; и трудно поддерживать, особенно для обеспечения того, чтобы общая часть соответствовала специализациям:
    • Вы используете какой-то вариант шаблона шаблонного метода : это позволяет вам учитывать общее поведение, используя дополнительные абстрактные методы, которые можно легко специализировать. Это менее избыточно, но дополнительное поведение имеет тенденцию становиться чрезвычайно разделенным. Слишком много значило бы, что это возможно слишком абстрактно.

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

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

Christophe
источник
5

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

public void sort(List values) {
    if (values instanceof LinkedList) {
        // do efficient linked list sort
    } else { // ArrayList
        // do efficient array list sort
    }
}

Я бы сделал это:

values.sort();

// ...

class ArrayList {
    public void sort() {
        // do efficient array list sort
    }
}

class LinkedList {
    public void sort() {
        // do efficient linked list sort
    }
}

Мы перемещаем поведение в место, которое знает, когда его использовать. Мы создаем реальную абстракцию, где вам не нужно знать типы или детали реализации. В вашей ситуации, возможно, имеет смысл переместить этот метод из исходного класса (который я буду называть O) для ввода Aи переопределения его в типе B. Если метод вызывается doItна какой - либо объект, перейти doItк Aи переопределение с различным поведением в B. Если есть биты данных, откуда doItон первоначально вызывается, или если метод используется в достаточном количестве мест, вы можете оставить исходный метод и делегировать:

class O {
    int x;
    int y;

    public void doIt(A a) {
        a.doIt(this.x, this.y);
    }
}

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

public void doIt(A a, boolean isTypeB) {
    if (isTypeB) {
        // do B stuff
    } else { 
        // do A stuff
    }
}

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

o.doIt(a, a instanceof B);

или:

o.doIt(a, true); //or false

Во-первых, точка вызова не знает, какой у Aнее тип . Следовательно, должны ли мы передавать логические значения полностью вниз? Это действительно шаблон, который мы хотим по всей базе кода? Что произойдет, если есть третий тип, который мы должны учитывать? Если это то, как метод вызывается, мы должны переместить его в тип и позволить системе полиморфно выбирать для нас реализацию.

Во-вторых, мы уже должны знать тип aв точке вызова. Обычно это означает, что мы либо создаем экземпляр там, либо принимаем экземпляр этого типа в качестве параметра. Создание метода, Oкоторый принимает Bздесь будет работать. Компилятор будет знать, какой метод выбрать. Когда мы проводим такие изменения, дублирование лучше, чем создание неправильной абстракции , по крайней мере до тех пор, пока мы не выясним, куда мы действительно идем. Конечно, я предполагаю, что мы на самом деле не закончили, что бы мы ни изменили к этому моменту.

Нам нужно более внимательно посмотреть на отношения между Aи B. Как правило, нам говорят, что мы должны отдавать предпочтение композиции, а не наследованию . Это не так в каждом случае, но это правда в удивительном количестве случаев, когда мы копаем. BНаследует A, что означает, что мы считаем, Bчто это A. Bследует использовать так же, как A, за исключением того, что он работает немного по-другому. Но каковы эти различия? Можем ли мы дать различиям более конкретное название? Разве это не Bесть A, но на самом деле Aесть, Xчто может быть A'или B'? Как бы выглядел наш код, если бы мы это сделали?

Если бы мы переместили метод Aв соответствии с предложенным ранее, мы могли бы внедрить экземпляр Xв Aи делегировать этот метод X:

class A {
    X x;
    A(X x) {
        this.x = x;
    }

    public void doIt(int x, int y) {
        x.doIt(x, y);
    }
}

Мы можем реализовать A'и B', и избавиться от B. Мы улучшили код, дав имя концепции, которая могла бы быть более неявной, и позволили себе устанавливать это поведение во время выполнения, а не во время компиляции. Aфактически стал менее абстрактным. Вместо расширенного отношения наследования он вызывает методы для делегированного объекта. Этот объект является абстрактным, но больше ориентирован только на различия в реализации.

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

B b = new B();
o.doIt(b, true);

Мы предполагали ранее, когда сочиняем, что Aимеет Xлибо, A'либо B'. Но, может быть, даже это предположение неверно. Это единственное место, где эта разница между Aи Bимеет значение? Если это так, то, возможно, мы можем использовать немного другой подход. У нас все еще есть, Xчто есть A'или B', но оно не принадлежит A. Только O.doItзаботится об этом, так что давайте только передать это O.doIt:

class O {
    int x;
    int y;

    public void doIt(A a, X x) {
        x.doIt(a, x, y);
    }
}

Теперь наш сайт звонков выглядит так:

A a = new A();
o.doIt(a, new B'());

Еще раз Bисчезает, и абстракция переходит в более сфокусированную X. На этот раз, однако, Aеще проще, зная меньше. Это еще менее абстрактно.

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

cbojar
источник
1
Меня поражает, что пример «плохого» кода, который вы даете здесь, похож на то, что я был бы склонен делать на неOO-языке. Интересно, усвоили ли они неправильные уроки и привели ли они в мир ОО как способ, которым они кодируют?
Baldrickk
1
@Baldrickk Каждая парадигма приносит свои собственные способы мышления со своими уникальными преимуществами и недостатками. В функционале Haskell лучшим вариантом будет сопоставление с образцом. Хотя на таком языке некоторые аспекты исходной проблемы также были бы невозможны.
cbojar
1
Это правильный ответ. Метод, который изменяет реализацию в зависимости от типа, с которым он работает, должен быть методом этого типа.
Роман Рейнер
0

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

Существует альтернатива: точки расширения , строгих абстракций и многоуровневой настройки. Скажите одну настройку государственных заказчиков, основанную на этой настройке для конкретного города.

Предупреждение: к сожалению, это работает лучше всего, когда все (или большинство) классов сделаны на продлен. Нет варианта для вас, может быть, в маленьком.

Эта расширяемость работает благодаря тому, что базовый класс расширяемого объекта содержит расширения:

void f(CreditorBO creditor) {
    creditor.as(AllowedCreditorBO.class).ifPresent(allowedCreditor -> ...);
}

Внутренне существует ленивое отображение объекта на расширенные объекты по классу расширения.

Для классов и компонентов GUI одинаковая расширяемость, частично с наследованием. Добавление кнопок и тому подобное.

В вашем случае проверка должна посмотреть, расширена ли она, и проверить себя на соответствие расширениям. Введение точек расширения только для одного случая добавляет непонятный код, не очень хорошо.

Таким образом, нет никакого решения, кроме попытки работать в текущем контексте.

Joop Eggen
источник
0

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

Абстракции хороши. Я ограничиваю их двумя принципами:

  • Лучше не абстрагироваться слишком рано. Подождите, пока больше примеров шаблонов, прежде чем абстрагироваться. «Больше», конечно, субъективно и специфично для сложной ситуации.

  • Избегайте слишком большого количества уровней абстракции только потому, что абстракция это хорошо. Программист должен будет держать эти уровни в своей голове для нового или измененного кода, поскольку они устанавливают кодовую базу и углубляются на 12 уровней. Желание хорошо абстрагированного кода может привести к такому количеству уровней, что многим людям будет трудно им следовать. Это также приводит к тому, что кодовые базы поддерживаются только ниндзя.

В обоих случаях «больше» и «слишком много» не являются фиксированными числами. Это зависит. Вот что затрудняет.

Мне также нравится это рецензия от Санди Мец

https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction

дублирование намного дешевле, чем неправильная абстракция,
и
предпочитайте дублирование неправильной абстракции

Майкл Даррант
источник