Неправильно ли использовать логический параметр для определения поведения?

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

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

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

151 голос | спросил Ray 10 Mayam12 2012, 02:33:30

16 ответов


76

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

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

Итак, что такое высокосвязная функция?

Это функция, которая делает только одно и только одно.

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

Было бы лучше отделить проблемы управления доступом от проблем Task, Action или Command.

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

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

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

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

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

ответил Rob 10 Maypm12 2012, 15:03:47
128

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

Но эта конкретная проблема легко фиксируется без принципиального изменения подхода:

 enum FrobnicationDirection {
  FrobnicateForwards,
  FrobnicateBackwards;
};

void frobnicate(Object what, FrobnicationDirection direction);

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

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

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

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

ответил James Youngman 10 Mayam12 2012, 03:02:00
33

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

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

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

Затем при вызове вы обычно вызываете foo(false) и foo(true) в зависимости от точного поведения, которое вы хотите.

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

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

 doThis();
doThat();

или

 doThis();

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

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

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

Существует хорошая статья от Мартина Фаулера , объясняющая более подробно ту же идею.

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

ответил Alex 10 Mayam12 2012, 05:53:53
22

Во-первых: программирование - это не наука, так как это искусство. Таким образом, очень редко «неправильный» и «правильный» способ программирования. Большинство стандартов кодирования - это просто «предпочтения», которые некоторые программисты считают полезными; но в конечном итоге они довольно произвольны. Поэтому я никогда бы не назвал выбор параметра «неправильным» сам по себе - и, конечно, не что-то общее и полезное как логический параметр. Использование кода boolean (или int, если на то пошло) для инкапсуляции состояния вполне оправдано во многих случаях.

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

ответил kmote 10 Mayam12 2012, 03:15:51
10

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

ответил dreza 10 Mayam12 2012, 00:26:48
7

Мне нравится подход к настройке поведения с помощью методов-конструкторов, которые возвращают неизменяемые экземпляры. Вот как Guava Splitter использует его:

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

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

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

  • Превосходная читаемость
  • Четкое разделение методов настройки и действий
  • Способствует сплочению, заставляя вас думать о том, что такое объект, что он должен и не должен делать. В этом случае это Splitter. Вы никогда не добавляли someVaguelyStringRelatedOperation(List<Entity> myEntities) в класс с именем Splitter, но вы подумали бы о его статическом методе в StringUtils.
  • Экземпляры предварительно сконфигурированы, поэтому они легко зависят от инъекций. Клиенту не нужно беспокоиться о том, следует ли передать true или false метод, чтобы получить правильное поведение.
ответил oksayt 10 Mayam12 2012, 06:40:29
6

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

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

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

Например:

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

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

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

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

ответил Jorn 10 Maypm12 2012, 16:39:06
3

Определенно запах кода . Если он не нарушает принцип единой ответственности , то он, вероятно, нарушает Скажите, не спрашивайте . Рассмотрим:

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

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

ответил Eva 6 MaramWed, 06 Mar 2013 03:30:16 +04002013-03-06T03:30:16+04:0003 2013, 03:30:16
2

Я удивлен, что никто не упомянул named-parameters .

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

 myObject.UpdateTimestamps(true);

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

 myObject.UpdateTimestamps(saveChanges: true);

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


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

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

//Two separate methods used
IEnumerable<Stuff> ascendingList = c.OrderBy(o => o.Key);
IEnumerable<Stuff> descendingList = c.OrderByDescending(o => o.Key); 
ответил BlueRaja - Danny Pflughoeft 10 Maypm12 2012, 20:59:45
1
  

Я не могу сформулировать, что в этом плохого.

Если он похож на запах кода, он чувствует себя как запах кода и хорошо пахнет запахом кода, это, вероятно, запах кода.

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

1) Избегайте методов с побочными эффектами.

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

ответил BaBu 10 Maypm12 2012, 14:18:10
0

Учитывая перспективу ОО, было бы лучше использовать разные реализации в зависимости от значения, рассчитанного вначале.

Но я не думаю, что это всегда пратматично.

ответил André Onuki 9 Maypm12 2012, 22:42:14
0

Это не обязательно неправильно, но в вашем конкретном примере действия, зависящем от какого-либо атрибута «пользователя», я бы передал ссылку на пользователя, а не на флаг.

Это разъясняет и помогает несколькими способами.

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

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

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

ответил James Anderson 10 Mayam12 2012, 06:00:22
0

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

Во-первых, если каждый вызов в последовательности имеет смысл сам по себе. Было бы разумно, если бы вызывающий код мог быть изменен с true на false или false на true, или , если вызываемый метод мог бы быть изменен на используйте параметр boolean напрямую, а не передавайте его. Вероятность десяти таких вызовов подряд небольшая, но это может произойти, и если бы это было сделано, это была бы хорошая практика программирования.

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

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

ответил RalphChapin 10 Maypm12 2012, 18:33:07
0

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

Я начал разрабатывать аппаратные средства в середине 70-х годов, которые мы теперь называем SCADA (диспетчерский контроль и сбор данных), и это были настроенные аппаратные средства с машинным кодом в EPROM, работающим с макросъемками и сбором высокоскоростных данных.

Логика называется Mealey и amp; Moore машины, которые мы называем сейчас Конечные машины . Они также должны выполняться в тех же правилах, что и выше, если только это машина в режиме реального времени с конечным временем выполнения, а затем для достижения цели должны быть сделаны ярлыки.

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

Но теперь с большим количеством памяти и легкостью OOK, инкапсуляция сегодня является существенным компонентом, но штраф, когда код подсчитывается в байтах для реального времени и машинного кода SCADA.

ответил Tony EE rocketscientist 11 Mayam12 2012, 02:35:36
0

TL; DR: не использовать логические аргументы.

См. ниже почему они плохие и , как их заменить (выделено жирным шрифтом).


Логические аргументы очень трудно читать, и поэтому их трудно поддерживать. Основная проблема заключается в том, что цель в целом очевидна, когда вы читаете сигнатуру метода, где указан аргумент. Однако присвоение имени параметру обычно не требуется на большинстве языков. Таким образом, у вас будут анти-шаблоны, такие как RSACryptoServiceProvider#encrypt(Byte[], Boolean) , где вызов определяет, какое шифрование должно использоваться.

Итак, вы получите такой вызов, как:

rsaProvider.encrypt(data, true);

, где читатель должен искать подпись метода просто для определения того, что может означать ад true. Передача целого числа, конечно, так же плохо:

rsaProvider.encrypt(data, 1);

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

Лучший способ решить это - использовать перечисление . Если вам необходимо передать enum RSAPadding двумя значениями: OAEP или PKCS1_V1_5), вы сразу сможете прочитать код:

rsaProvider.encrypt(data, RSAPadding.OAEP);

Булевы могут иметь только два значения. Это означает, что если у вас есть третий вариант, вам придется реорганизовать свою подпись. Как правило, это невозможно сделать из-за обратной совместимости, поэтому вам придется расширить любой открытый класс другим общедоступным методом. Это то, что Microsoft наконец сделала, когда они представили RSACryptoServiceProvider#encrypt(Byte[], RSAEncryptionPadding) , где они использовали перечисление (или, по крайней мере, класс, имитирующий перечисление) вместо логического.

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


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

Определение заполнения - это то, что выполняется во время создания или инициализации экземпляра класса. Таким образом, код может выйти из строя до выбран алгоритм заполнения. Например, многие ключи RSA на смарт-картах поддерживают только один из двух дополняющих механизмов. Таким образом, вы можете создать поставщика и узнать только, что механизм не поддерживается, когда вы выполняете фактическое шифрование или дешифрование. Основополагающая философия называется fail-fast .


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

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


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


Почти все гуру программного обеспечения, которые мне нравятся, предупреждают о логических аргументах. Например, Джошуа Блох предупреждает против нихвысоко ценная книга «Эффективная Ява». В общем, их просто не следует использовать. Вы можете утверждать, что их можно использовать, если в том случае, когда есть один параметр, который легко понять. Но даже тогда: Optional, вероятно, лучше реализовано с использованием двух методов : Bit.set(Boolean) и Bit.set().


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

Bit.unset()

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

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

...

cipher.init(key, ENCRYPT);

, даже если вы предпочитаете:

cipher.init(key, true);

вместо.

ответил Maarten Bodewes 11 Maypm18 2018, 21:47:40
-2

Контекст важен. Такие методы довольно распространены в iOS. Как только один часто используемый пример, UINavigationController предоставляет метод -pushViewController:animated:

ответил Caleb 10 Mayam12 2012, 03:05:50

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132