Это неправильно использовать логический параметр, чтобы определить поведение?

Я видел практику время от времени, что "чувствует" и так, но я могу'т достаточно четко сформулировать, что именно не так. Или, может быть, это's просто мое предубеждение. Здесь идет:

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

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

Комментарии к вопросу (6)

Я перестал использовать этот шаблон давным-давно, по очень простой причине; расходы на техническое обслуживание. Несколько раз я обнаружил, что некоторые функции сказать frobnicate(что-то, forwards_flag), который был вызван много раз в моем коде, и нужно найти все места в коде, где значениеfalseбыл принят в качестве значенияforwards_flag`. Вы можете'т удобный поиск для тех, так что это становится головной болью обслуживания. И если вам нужно сделать исправление на каждом из этих сайтов, вы можете иметь досадная проблема, если вы пропустите один.

Но данная проблема легко решается без коренного изменения подхода:

enum FrobnicationDirection {
  FrobnicateForwards,
  FrobnicateBackwards;
};

void frobnicate(Object what, FrobnicationDirection direction);

С этот код, только нужно искать экземпляры FrobnicateBackwards. В то время как он's возможно, есть какой-то код, который присваивает этой переменной, так что вы должны соблюдать некоторые нити управления, я считаю, что на практике это достаточно редко, что этот вариант работает нормально.

Однако, есть еще одна проблема с прохождением флаг таким образом, по крайней мере в принципе. Это то, что некоторые (только некоторые) системах, эта конструкция может быть подвергая слишком много знаний о сведениях реализации многоуровневых части кода (который использует флаг) в наружные слои (которые нужно знать какое значение следует пройти в этот флаг). Использовать Ларри Константин'с терминологией, эта конструкция может иметь более сильный <а href="и http://en.wikipedia.org/wiki/Coupling_(computer_programming)" и>соединение</а> между сеттер и пользователь булев флаг. Фрэнки, хотя он'ы трудно сказать с какой-либо степенью определенности на этот вопрос, не зная больше о коде.

Рассмотреть конкретные примеры вы даете, я бы некоторым степень озабоченности в каждом, но в основном по причинам риска/правильности. То есть, если ваша система должна проходить вокруг флагов, которые указывают, в каком состоянии система находится в, вы можете обнаружить, что вы'ве получил код, которые должны учитывать это, но не'т проверить параметр (поскольку он не был передан в эту функцию). Так что у вас есть ошибка, потому что кто-то забыл передать параметр.

Это's также стоит признать, что система-индикатор состояния, который должен быть передан почти каждая функция-это по сути глобальная переменная. Многие из недостатков глобальной переменной будет применяться. Я думаю, что во многих случаях лучше практика, чтобы инкапсулировать знание состояния системы (или пользователя'ы учетных данных, или система'ы личность) в пределах объекта, который обязан действовать правильно на основе этих данных. Затем проходите по ссылки на этот объект в отличие от исходных данных. Ключевое понятие здесь - <а href="(object-orientedprogramming)&quot http://en.wikipedia.org/wiki/Encapsulation;>encapsulation.

Комментарии (3)
Решение

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

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

Так что это высоко-когезивный функция?

Это's в функцию, которая делает только одну вещь и одну вещь.

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

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

Как вы уже отметили, переплетение этих проблем, кажется, выкл.

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

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

Я бы полностью избавиться от данного параметра. Имея возможность отключения контроля доступа даже для тестирования представляет потенциальный риск для безопасности. Для целей тестирования либо [тэг:заглушка] или [метки:макет] проверку доступа для проверки и доступ разрешен и доступ запрещен сценарии.

Реф: сплоченности (информатика)

Комментарии (5)

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

Базовый сценарий, которого следует избегать относительно булевых параметров:

в

public void foo(boolean flag) {
    doThis();
    if (flag)
        doThat();
}

Затем, когда вам звонит'д обычно называют фу(ложно) " и " фу(правда) в зависимости от поведения вы хотите.

Это действительно проблема, потому что она's в случае плохой сплоченности. Вы'вновь создавая зависимость между методами, что это не нужно.

Что вы должны делать в этом случае уходит сделать и сделать как отдельные и общие методы, то делаем:

doThis();
doThat();

или

doThis();

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

Конечно, не все логические параметры используются в такую плохую сторону, но это'ы наверняка это код запах, и вы'вновь право получить подозрительно, если вы видите, что много в исходном коде.

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

Есть хорошая статья От Мартина Фаулера]2, объясняя в подробности та же идея.

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

Комментарии (9)

Во-первых: программирование-это не наука, так как это искусство. Поэтому там очень редко а "неправильно" и "Мои права" в пути к программе. Большинство кодинг-стандартов просто на "предпочтения", что некоторые программисты считают полезным; но в конечном счете они достаточно произвольны. Поэтому я никогда бы не метка выбор параметра, чтобы быть "неправильно" в себе ... и, конечно, не то, как универсальные и полезные в качестве параметра булево. Использование логическое (или инт, если на то пошло), чтобы инкапсулировать состояние вполне оправдано во многих случаях.

Решения кодирования, по & большие, должны быть основаны в первую очередь на производительность и ремонтопригодность. Если производительность-это'т на кону (и я могу'т представьте себе, как это может быть в ваших примерах), то далее следует: насколько легко это будет для меня (или будущий редактор) для поддержания? Он интуитивен и понятен? Он изолирован? Ваш пример цепочки вызовов функций действительно кажется, в этом отношении потенциально хрупкий: если вы решили изменить ваш bIsUp до bIsDown, как во многих других местах в коде нужно будет тоже менять? Кроме того, ваш список параметр воздухоплавание? Если ваша функция имеет 17 параметров, то читабельность является проблемой, и вы должны пересмотреть ли вы, оценив преимущества объектно-ориентированной архитектуры.

Комментарии (3)

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

Комментарии (2)

Я думаю, самое главное здесь, чтобы быть практичным.

Когда boolean определяет все поведение, просто сделать второй способ.

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

Например:

в

private void FooInternal(bool flag)
{
  //do work
}

public void Foo1()
{
  FooInternal(true);
}

public void Foo2()
{
  FooInternal(false);
}

Конечно, на практике вы'll всегда есть точка между этими крайностями. Обычно я просто идти с тем, что чувствует себя хорошо, но я предпочитаю ошибаться в сторону менее дублирования кода.

Комментарии (2)

Мне нравится подход к настройке поведения с помощью методов застройщика, которые возвращают непреложный экземпляров. Здесь's, как гуава сплиттер использует его:

в

private static final Splitter MY_SPLITTER = Splitter.on(',')
       .trimResults()
       .omitEmptyStrings();

MY_SPLITTER.split("one,,,,  two,three");

Преимуществами этого являются:

  • Улучшенный читаемость
  • Четкое разделение конфигурации и методов действий
  • Способствует сплоченности, заставляя вас думать о том, что объект, для чего это надо и надо'т сделать. В данном случае это'ы сплиттер. Вы'd никогда бы не поставил someVaguelyStringRelatedOperation(список<лицо> myEntities)в класс - разделитель`, но вы'd не думаю, о сдаче его в качестве статического метода в StringUtils класс.
  • Экземпляры предварительно настроенных поэтому легко зависимость-инъекций. Клиент не'т нужно беспокоиться о том, чтобы передать true или false, чтобы метод, чтобы получить правильное поведение.
Комментарии (3)

ТЛ;ДР: Дон'т использовать логические аргументы.

См. ниже почему они плохие, и как их заменить* (жирным лицом).


Логические рассуждения очень трудно читать, и, таким образом, трудно поддерживать. Главная проблема заключается в том, что цель в целом понятна, когда читаешь сигнатуру метода, где аргумент называется. Однако, называя параметр, как правило, не требуется в большинстве языков. Так что вы будете иметь анти-паттерны, как rsacryptoserviceprovider с [#шифрование(байт[], boolean) с](https://msdn.microsoft.com/en-us/library/f17a0e2k(в=против 110).аспн), где параметр типа boolean определяет, какой вид шифрования используется в функции.

Так вы получите позвоните, как:

rsaProvider.encrypt(data, true);

где читатель должен найти способ'подпись просто определить какого черта "истина" действительно может означать. Передача целого числа-это, конечно, так же плохо:

rsaProvider.encrypt(data, 1);

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

Лучший способ решить это, чтобы использовать перечисления. Если вы должны пройти перечислимый RSAPadding с двумя значениями: УАМИАЭ " или " PKCS1_V1_5 тогда вы бы сразу быть в состоянии прочитать код:

rsaProvider.encrypt(data, RSAPadding.OAEP);

Булевы переменные могут иметь только два значения. Это означает, что если у вас третий вариант, то вы'd должны переработать свой подписью. Как правило, это не может быть легко выполнена, если обратная совместимость-это вопрос, так что вы'd имеют для расширения любого общественного класса другим государственным методом. Это то, что Microsoft, наконец, сделали, когда они ввели rsacryptoserviceprovider с [#шифровать(байт[], RSAEncryptionPadding)](https://msdn.microsoft.com/en-us/library/mt132683(в=против 110).аспн), где они использовали перечисления (или, по крайней мере класса имитируя перечисление), а не логическое.

Очень даже может быть легче использовать полный объект или интерфейс в качестве параметра, в случае, если сам параметр должен быть параметризован. В приведенном выше примере УАМИАЭ сама обивка может быть параметризована с хэш-значения для внутреннего использования. Обратите внимание, что есть сейчас 6 алгоритм SHA-2 хэш-алгоритмов и 4 ша-3 хэш-алгоритмы, поэтому количество значений перечисления могут взорваться, если вы используете только одно перечисление, а не параметров (это, возможно, следующее, что Microsoft собирается выяснить).


Логических параметров может также указать, что метод или класс-это не хорошо разработана. Как и в приведенном выше примере: любые криптографические библиотеки, чем другие .Чистая никто не'т использовать флаг, перетяжка в сигнатуру метода на всех.


Почти все программное обеспечение гуру, которые мне нравятся предостерегать от логических аргументов. Например, Джошуа блох предупреждает их высоко оценили книгой "Эффективная Java и". В общем они просто не должны использоваться. Можно утверждать, что они могут быть использованы, если дело, что есть один параметр, который легко понять. Но даже тогда: бит.набор(логическое значение) - это, наверное, лучше реализовать с помощью **двух способов**:бит.набор () " и " бит.и unset()`.


Если вы не можете напрямую изменить код можно определение констант по крайней мере, сделать их более читабельным:

const boolean ENCRYPT = true;
const boolean DECRYPT = false;

...

cipher.init(key, ENCRYPT);

это гораздо более читабельным, чем:

cipher.init(key, true);

даже если вы'д, а есть:

cipher.initForEncryption(key);
cipher.initForDecryption(key);

вместо.

Комментарии (0)

Безусловно, код. Если это не'т нарушать единый принцип ответственности, то это, вероятно, нарушает скажи, Дон'т спросить. Рассмотрим:

Если получится не нарушить один из этих двух принципов, вы все равно должны использовать перечислимый. Булевские флаги являются логическим эквивалентом магия чисел. фу(ложные) делает столько же смысла, какбар(42)`. Перечисления могут быть полезны для шаблон стратегия и обладает гибкостью, позволяя вам добавить другую стратегию. (Только не забудьте имя их надлежащим образом.)

Ваш конкретный пример особенно беспокоит меня. Почему этот флаг прошел через так много методов? Это звучит, как вы должны разделить параметр на подклассы](http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism).

Комментарии (0)

Я'м удивлен, никто не упомянул именованные параметры.

Проблему я вижу с булевых флагов, что они вредят читабельности. Например, что значит "истина" в

в

myObject.UpdateTimestamps(true);

делать? Я понятия не имею. Но как насчет:

myObject.UpdateTimestamps(saveChanges: true);

Теперь это's не понятно, какой параметр мы'повторного прохождения должна делать: мы'вновь рассказывал функцию, чтобы сохранить свои изменения. В этом случае, если класс является непубличным, я думаю, что параметр типа boolean нормально.


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

//Enum used
double a = Math.Round(b, MidpointRounding.AwayFromZero);

//Two separate methods used
IEnumerable ascendingList = c.OrderBy(o => o.Key);
IEnumerable descendingList = c.OrderByDescending(o => o.Key); 
Комментарии (5)

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

Я начал проектирование оборудования в середине 70's, который мы сейчас называем SCADA (диспетчерское управление и сбор данных) и они были хорошо настроены аппаратного обеспечения с машинных кодов в ПЗУ выполняется макрос дистанционного управления и сбора данных высокой скорости.

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

Данные синхронно, но команды выполняются асинхронно и командной логики следует низкозатратная булевой логики, но с последовательным команды, основанные на памяти о прошлых, настоящих и желаемого состояния. Для того, чтобы функционировать в наиболее эффективный машинный язык (только 64 КБ), большую заботу был взят, чтобы определить каждый процесс в эвристической IBM в рамках моды. Так что иногда, проходя логические переменные и делаешь проиндексированных филиалы.

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

Комментарии (0)

Я могу'т достаточно четко сформулировать, что именно не так.

Если это выглядит как код запах, похоже на запах код и - хорошо пахнет запах код, он's наверное это код запах.

Что вы хотите сделать, это:

  1. не допускать методы с побочными эффектами.

  2. обрабатывать необходимую государств Центральной, официальной государственной машины (как это).

Комментарии (0)

Ее не обязательно плохо, но в ваш конкретный пример действий в зависимости от некоторых атрибутов от "пользователей" Я бы пройти через ссылку на пользователя, а не флаг.

Это проясняет и помогает в ряде способами.

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

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

Если одну функцию/метод в "цепочке" и делает что-то другое в зависимости от атрибута пользователя, это очень вероятно, что аналогичная зависимость от атрибутов пользователя будут представлены некоторые из других методов в "цепочке" по.

Комментарии (0)

Большую часть времени, я считаю, что это плохо кодирования. Я могу думать о двух случаях, где это может быть хорошей практикой. Как много ответов уже говорил, почему это's плохой, я предлагаю два раза, когда это может быть хорошо:

В первом случае при каждом вызове в последовательности имеет смысл в своем собственном праве. Это имело бы смысл, если вызывающий might код будет изменить с True на false или false на True, or если вызываемый метод might быть изменен с помощью параметра логическое напрямую, а не передать его. Вероятность десяти таких звонков мало, но это может произойти, и если это действительно было бы хорошей практикой программирования.

Второй случай немного растянуть, учитывая, что мы'вновь имеем дело с логическим. Но если программа имеет несколько потоков или событий, передача параметров-это самый простой способ, позволяющий отслеживать поток/конкретные данные события. Например, программа может получать входные данные из двух или более розеток. Код работает для одного сокета может понадобиться для получения предупреждающих сообщений, и один на один не может. Затем он (вроде) смысл для логического набора на очень высоком уровне, чтобы прошли через множество вызовов к местам, где предупреждающие сообщения могут быть сгенерированы. Данные могут'т быть сохранены (за исключением с большим трудом) в какой-то глобальный, потому что несколько потоков или события чередуются бы каждому нужна своя ценность.

Надо отметить, что в последнем случае я'д, наверное, создать класс/структуру, только содержание которых было логическое и передать девчонка вокруг вместо. Я'd быть почти наверняка нужно, прежде чем слишком долго ... как where отправлять предупреждающие сообщения, например, в других сферах.

Комментарии (1)

Контекст важен. Такие методы довольно распространены в iOS. Как только один часто используемый пример, UINavigationController предоставляет метод -pushViewController:анимированные:, и анимированные параметр типа bool. Метод, по сути, выполняет ту же функцию в любом случае, но это оживляет переход от одного контроллера представления к другому, если вы проходите в да, и не't если вы проходите нет. Это представляется вполне разумным; он'd быть глупым, чтобы обеспечить двумя способами в место этого, чтобы ты мог определить, является ли или не использовать анимации.

Это может быть легче оправдать такого рода вещи в Objective-C, где метод именования синтаксис обеспечивает более широкий контекст для каждого параметра, чем в таких языках, как C и Java. Тем не менее, я'd не думаю, что метод, который принимает один параметр может легко принять логическое и еще есть смысл:

file.saveWithEncryption(false);    // is there any doubt about the meaning of 'false' here?
Комментарии (13)