В некотором коде, который я рассматриваю, я вижу вещи, которые морально эквивалентны следующему:
public class Foo
{
private Bar bar;
public MethodA()
{
bar = new Bar();
bar.A();
bar = null;
}
public MethodB()
{
bar = new Bar();
bar.B();
bar = null;
}
}
Поле bar
здесь является логически локальной переменной, так как его значение никогда не должно сохраняться при вызовах методов. Однако, поскольку многие методы Foo
нуждаются в объекте типа Bar
, автор исходного кода только что создал поле типа Bar
.
- Это явно плохо, правда?
- Есть ли название для этого антипаттерна?
object-oriented
terminology
anti-patterns
JSB ձոգչ
источник
источник
MethodA
илиMethodB
вызываетMethodA
илиMethodB
будет вызван каким-либо образом (иbar
используется снова, в случаеbar.{A,B}()
, если вы являетесь нарушителем), у вас возникают подобные проблемы даже без какого-либо параллелизма.Ответы:
Да.
Это делает методы не реентерабельными, что является проблемой, если они вызываются в одном и том же экземпляре рекурсивно или в многопоточном контексте.
Это означает, что состояние от одного вызова просачивается к другому вызову (если вы забыли повторно инициализировать).
Это делает код трудным для понимания, потому что вы должны проверить вышеизложенное, чтобы убедиться, что код на самом деле делает. (Контраст с использованием локальных переменных.)
Это делает каждый
Foo
экземпляр больше, чем нужно. (И представьте, что вы делаете это для N переменных ...)ИМО, это не стоит называть антипаттерном. Это просто плохая привычка кодирования / неправильное использование Java-конструкций / дерьмового кода. Это то, что вы можете увидеть в коде студента, когда студент пропускает лекции и / или не имеет навыков программирования. Если вы видите это в рабочем коде, это признак того, что вам нужно сделать намного больше обзоров кода ...
Для записи, правильный способ написать это:
Обратите внимание, что я также исправил имена методов и переменных, чтобы они соответствовали принятым правилам стиля для идентификаторов Java.
источник
Я бы назвал это ненужной большой областью действия для переменной. Этот параметр также учитывает условия гонки, если несколько потоков обращаются к MethodA и MethodB.
источник
Это частный случай общего паттерна, известного как
global doorknobbing
. При строительстве дома заманчиво купить только одну дверную ручку и оставить ее лежащей без дела. Когда кто-то хочет использовать дверь или шкаф, он просто берет эту глобальную ручку двери. Если дверные ручки стоят дорого, это может быть хорошим дизайном.К сожалению, когда приходит ваш друг и дверная ручка одновременно используется в двух разных местах, реальность разрушается. Если дверная ручка настолько дорогая, что вы можете позволить себе только одну, то стоит создать систему, которая безопасно позволит людям ждать своей очереди за дверной ручкой.
В этом случае дверная ручка - это просто ссылка, поэтому она дешевая. Если дверная ручка была реальным объектом (и они повторно использовали объект, а не просто ссылку), то это может быть достаточно дорого, чтобы быть
prudent global doorknob
. Однако, когда это дешево, это известно какcheapskate global doorknob
.Ваш пример
cheapskate
разнообразен.источник
Основной проблемой здесь будет параллелизм - если Foo используется несколькими потоками, у вас есть проблема.
Кроме того, это просто глупо - локальные переменные прекрасно управляют своими собственными жизненными циклами. Замена их переменными экземпляра, которые должны быть аннулированы, когда они больше не нужны, является приглашением к ошибке.
источник
Это особый случай «неправильного определения области действия» со стороной «повторного использования переменной».
источник
Это не анти-паттерн. Анти-паттерны обладают некоторым свойством, которое делает их хорошей идеей, которая заставляет людей делать это нарочно; они запланированы как образцы, и тогда это идет ужасно неправильно.
Это также то, что вызывает споры относительно того, является ли что-то шаблоном, анти-шаблоном или часто неправильно применяемым шаблоном, который все еще имеет применение в некоторых местах.
Это просто неправильно.
Чтобы добавить немного больше.
Этот кодекс является суеверным или, в лучшем случае, практикой культа грузов.
Суеверие - это то, что делается без чёткого оправдания. Это может быть связано с чем-то реальным, но связь не логична.
Практика культа грузов - это та, в которой вы пытаетесь скопировать то, что вы узнали из более осведомленного источника, но вы на самом деле копируете поверхностные артефакты, а не процесс (так называемый культ в Папуа-Новой Гвинее, который радиоуправляемые самолеты из бамбука в надежде вернуть самолеты Второй мировой войны в Японию и США).
В обоих этих случаях нет никакого реального случая.
Анти-паттерн - это попытка разумного улучшения, будь то в малом (это дополнительное ветвление для решения того дополнительного случая, который должен быть обработан, который приводит к коду спагетти) или в большом, где вы очень сознательно реализуете паттерн это либо дискредитируется, либо обсуждается (многие описывают синглтоны как таковые, некоторые исключают только запись - например, регистрируют объекты или только чтение - например, объекты настроек конфигурации - а некоторые осуждают даже те), или где вы решаете неправильная проблема (когда впервые был выявлен .NET, MS рекомендовала шаблон для утилизации, когда у вас были как неуправляемые поля, так и одноразовые управляемые поля - это действительно очень хорошо справляется с этой ситуацией, но реальная проблема заключается в том, что у вас есть оба типа поля в одном классе).
Таким образом, анти-паттерн - это то, что умный человек, который хорошо знает язык, проблемную область и доступные библиотеки, намеренно сделает это, у которого все еще есть (или, как утверждается, есть) недостаток, который подавляет потенциал.
Поскольку никто из нас не начинает хорошо знать данный язык, проблемную область и доступные библиотеки, и поскольку каждый может что-то упустить при переходе от одного разумного решения к другому (например, начать что-то хранить в поле для хорошего использования, а затем попытаться проведите рефакторинг, но не завершите работу, и вы получите код, как в вопросе), и поскольку мы все время от времени пропускаем что-то в процессе обучения, мы все в какой-то момент создали какой-то суеверный или культовый код , Хорошо, что на самом деле их легче идентифицировать и исправить, чем анти-паттерны. Истинные антишаблоны либо спорно не антишаблоны, или иметь некоторые привлекательные качества, или, по крайней мере, иметь какой-то способ заманивания один в них даже тогда, когда определены как плохие (слишком много и слишком мало слоев как неоспоримому плохо, но избежать одного приводит к другому).
источник
(1) Я бы сказал, что это не хорошо.
Он выполняет ручное хранение, что стек может делать автоматически с локальными переменными.
Выигрыш в производительности отсутствует, поскольку «new» вызывается при каждом вызове метода.
РЕДАКТИРОВАТЬ: объем памяти класса больше, потому что бар указатель всегда занимает память в течение жизни класса. Если бы указатель панели был локальным, то он использовал бы только память на время существования вызова метода. Память для указателя - это просто капля в море, но это все еще ненужная капля.
Панель видна другим методам в классе. Методы, которые не используют бар, не должны видеть его.
Ошибки на основе состояния. В этом конкретном случае ошибки базы состояний должны возникать только при многопоточности, поскольку каждый раз вызывается «new».
(2) Я не знаю, есть ли название для него, поскольку на самом деле это несколько вопросов, а не одна.
- ручное ведение домашнего хозяйства, когда доступно автоматическое -
ненужное состояние - состояние, которое
длится дольше, чем его срок службы -
видимость или область действия слишком высоки
источник
Я бы назвал это «Блуждающий ксенофоб». Он хочет, чтобы его оставили в покое, но он не знает, где и что это такое. Таким образом, это плохо, как утверждают другие.
источник
В данном случае идет вразрез, но хотя я не считаю данный пример хорошим стилем кодирования, как в его нынешнем виде, шаблон существует во время модульного тестирования.
Это довольно стандартный способ проведения юнит-тестирования
это просто рефакторинг этого эквивалента кода OP
Я бы никогда не написал код, указанный в примере с OP, он кричит рефакторинг, но я считаю, что шаблон не является ни недействительным, ни антипаттерном .
источник
bar
значениеnull
, JUnit в любом случае создает новый экземпляр теста для каждого метода тестирования.Этот запах кода упоминается как Временное поле при рефакторинге Беком / Фаулером.
http://sourcemaking.com/refactoring/temporary-field
Отредактировано, чтобы добавить дополнительные пояснения по запросу модераторов: Вы можете узнать больше о запахе кода в указанном выше URL-адресе или в печатном экземпляре книги. Поскольку ОП искал название для этого специфического запаха антипаттерна / кода, я подумал, что было бы полезно сослаться на ранее не упомянутую ссылку на него из авторитетного источника, которому более 10 лет, хотя я полностью понимаю, что опаздываю к игра по этому вопросу, так что вряд ли за ответ проголосуют или примут.
Если это не слишком личная реклама, я также упоминаю и объясняю этот специфический запах кода в моем предстоящем курсе Pluralsight «Рефакторинг Основы», который, как я ожидаю, будет опубликован в августе 2013 года.
Чтобы добавить больше ценности, давайте поговорим о том, как исправить эту конкретную проблему. Есть несколько вариантов. Если поле действительно используется только одним методом, его можно преобразовать в локальную переменную. Однако, если несколько методов используют поле для связи (вместо параметров), то это классический случай, описанный в Рефакторинге, и его можно исправить, заменив поле параметрами, или если методы, работающие с этим кодом, тесно related, Extract Class может использоваться для извлечения методов и полей в их собственный класс, в котором поле всегда находится в допустимом состоянии (возможно, заданном во время конструирования) и представляет состояние этого нового класса. Если вы идете по маршруту параметра, но слишком много значений для передачи, другой вариант - ввести новый объект параметра,
источник
Я бы сказал, что когда вы к этому приступите, это действительно отсутствие сплоченности, и я могу дать ему название «Ложная сплоченность». Я бы назвал этот термин (или, если я получаю его откуда-то и забыл об этом, «заимствовал»), потому что класс кажется связным в том смысле, что его методы работают с полем-членом. Однако на самом деле это не так, что означает, что кажущаяся сплоченность фактически ложна.
Что касается того, плохо это или нет, я бы сказал, что это ясно, и я думаю, что Стивен С. отлично объясняет почему. Тем не менее, независимо от того, заслуживает ли оно того, чтобы его называли «анти-паттерном» или нет, я думаю, что это и любая другая парадигма кодирования могут быть связаны с меткой, поскольку это обычно облегчает общение.
источник
Помимо уже упомянутых проблем, использование этого подхода в среде без автоматической сборки мусора (например, C ++) приведет к утечкам памяти, поскольку временные объекты не освобождаются после использования. (Хотя это может показаться немного надуманным, есть люди, которые просто поменяют 'null' на 'NULL' и будут рады, что код компилируется.)
источник
Это выглядит для меня как ужасное неправильное применение шаблона Flyweight. К сожалению, я знал нескольких разработчиков, которые должны использовать одну и ту же «вещь» несколько раз в разных методах и просто вытаскивать ее в приватное поле, пытаясь сбить с толку память, улучшить производительность или какую-то другую странную псевдооптимизацию. По их мнению, они пытаются решить аналогичную проблему, поскольку Flyweight предназначен для решения только они делают это в ситуации, когда это на самом деле не проблема, которая требует решения.
источник
Я сталкивался с этим много раз, обычно при обновлении кода, ранее написанного кем-то, кто выбрал VBA For Dummies в качестве своего первого названия и продолжал писать относительно большую систему на любом языке, на котором они наткнулись.
Сказать, что это действительно плохая практика программирования, правильно, но, возможно, это просто результат того, что интернет-ресурсы и ресурсы, прошедшие экспертную оценку, такие, которые нам всем нравятся сегодня, могли бы привести первоначального разработчика к более «безопасному» пути.
Хотя на самом деле он не заслуживает тега «antipattern», но было приятно увидеть предложения о том, как можно перекодировать OP.
источник