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

39

Согласно Неправильно ли использовать логический параметр для определения поведения? Я знаю важность избегания использования логических параметров для определения поведения, например:

оригинальная версия

public void setState(boolean flag){
    if(flag){
        a();
    }else{
        b();
    }
    c();
}

новая версия:

public void setStateTrue(){
    a();
    c();
}

public void setStateFalse(){
    b();
    c();
}

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

public void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

Я пытаюсь устранить флаг isHintOn и создать 2 отдельные функции:

public void setHintOn(){
    this.layer1.visible=true;
    this.layer2.visible=false;
    this.layer3.visible=true;
}

public void setHintOff(){
    this.layer1.visible=false;
    this.layer2.visible=true;
    this.layer3.visible=false;
}

но модифицированная версия кажется менее ремонтопригодной, потому что:

  1. в нем больше кодов, чем в оригинальной версии

  2. это не может ясно показать, что видимость layer2 противоположна опции подсказки

  3. когда новый слой (например: layer4) добавлен, мне нужно добавить

    this.layer4.visible=false;
    

    а также

    this.layer4.visible=true;  
    

    в setHintOn () и setHintOff () отдельно

Поэтому у меня вопрос: если булев параметр используется только для определения значений, но не для поведения (например, нет в этом параметре if-else), рекомендуется ли по-прежнему исключать этот булев параметр?

ocomfd
источник
26
Это никогда не неправильно, если полученный код более читабелен и удобен в обслуживании ;-) Я бы порекомендовал использовать один метод вместо двух отдельных методов.
helb
32
Вы приводите убедительный аргумент, что единственная реализация метода, который устанавливает эти логические значения, приведет к упрощению обслуживания класса и понимания его реализации. Отлично; это законные соображения. Но публичный интерфейс класса не должен быть деформирован, чтобы приспособиться к ним. Если отдельные методы облегчат понимание и работу с открытым интерфейсом, определите ваш setHint(boolean isHintOn)как частный метод и добавьте открытый setHintOnи setHintOffметоды, которые соответственно вызывают setHint(true)и setHint(false).
Марк Амери
9
Я был бы очень недоволен этими именами методов: они действительно не предлагают никакой выгоды setHint(true|false). Картофельные потахто. По крайней мере, использовать что-то вроде setHintи unsetHint.
Конрад Рудольф
5
Возможный дубликат Лучше Show () + Hide () или SetVisible (bool visible)?
17 из 26
4
@kevincline Если условием является одно имя, вы пишете isв начале. isValidи т.д. Так зачем менять это на два слова? Кроме того, «более естественный» в глазах смотрящего. Если вы хотите произнести это как английское предложение, то для меня было бы более естественным иметь «если подсказка включена» с «заправленной» буквой.
Мистер Листер

Ответы:

95

Разработка API должна быть сосредоточена на том, что наиболее полезно для клиента API, с вызывающей стороны .

Например, если этот новый API требует от вызывающего абонента регулярно писать такой код

if(flag)
    foo.setStateTrue();
else
    foo.setStateFalse();

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

 foo.setState(flag);

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

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

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

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

Это работает и наоборот: если setStateвышеуказанному методу нужно переключаться между двумя разными частями кода, эти части кода могут быть реорганизованы в два разных закрытых метода. Поэтому ИМХО не имеет смысла искать критерий для выбора между «один параметр / один метод» и «нулевые параметры / два метода», просматривая внутренние компоненты. Однако посмотрите, как бы вы хотели видеть API в роли его потребителя.

Если вы сомневаетесь, попробуйте использовать «разработку через тестирование» (TDD), которая заставит вас задуматься об общедоступном API и о том, как его использовать.

Док Браун
источник
DocBrown, вы бы сказали, что подходящей разделительной линией является то, имеет ли настройка каждого состояния сложные и, возможно, необратимые побочные эффекты? Например, если вы просто переключаете флаг, который делает то, что он говорит на жестяной коробке, и нет конечного автомата, вы должны параметризовать различные состояния. Принимая во внимание, что, например, вы не станете параметризовать такой метод SetLoanFacility(bool enabled), потому что, предоставив кредит, может быть нелегко забрать его снова, и эти два варианта могут включать в себя совершенно разную логику - и вы захотите перейти к отдельному Create Удалить методы.
Стив
15
@Steve: вы все еще пытаетесь спроектировать API исходя из требований, которые вы видите на стороне реализации. Быть прямым: это совершенно не имеет значения. Используйте тот вариант публичного API, который проще использовать с вызывающей стороны. Внутренне вы всегда можете позволить двум открытым методам вызывать один закрытый с параметром. Или наоборот, вы можете позволить одному методу с параметром переключаться между двумя приватными методами с различной логикой.
Док Браун
@Steve: см. Мое редактирование.
Док Браун
Я беру все ваши замечания - я на самом деле думаю об этом со стороны вызывающей стороны (отсюда моя ссылка на «то, что написано на банке») и пытаюсь сформулировать соответствующее правило, в котором вызывающая сторона обычно ожидает использовать каждый подход. Мне кажется, что правило состоит в том, ожидает ли вызывающий абонент повторных вызовов как идемпотентных, а переходы состояний - без ограничений и без сложных побочных эффектов. Параметрирование включения и выключения комнатного освещения будет параметризовано, включение и выключение региональной электростанции будет многоцелевым.
Стив
1
@Steve Так что, если пользователю нужно подтвердить определенную настройку, то Toggle()это неправильная функция для предоставления. Вот и весь смысл; если вызывающий абонент заботится только о «изменить его», а не о том, «чем он заканчивается», то Toggle()это вариант, позволяющий избежать дополнительной проверки и принятия решения. Я бы не назвал это ОБЩИМ случаем, и при этом я не рекомендовал бы сделать это доступным без уважительной причины, но если пользователю нужен переключатель, то я бы дал им переключение.
Камиль Дракари
40

Мартин Фаулер цитирует Кента Бека, который рекомендует отдельные setOn() setOff()методы , но также говорит, что это не следует считать неприкосновенным:

Если вы потянув [так] данные из булева источника, например, управления пользовательского интерфейса или источника данных, я бы предпочел , setSwitch(aValue)чем

if (aValue)
  setOn();
else
  setOff();

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

Другая рекомендация состоит в том, чтобы использовать перечисляемое значение или тип флагов, чтобы дать trueи falseболее точные, зависящие от контекста имена. В твоем примере showHintи hideHintмогло быть и лучше.

Грэм Ли
источник
16
По моему опыту, setSwitch (значение) почти всегда приводит к меньшему общему коду, чем setOn / setOff именно из-за кода if / else, указанного в вашем ответе. Я склонен проклинать разработчиков, которые дают мне API setOn / setOff, а не setSwitch (value).
17 из 26
1
Глядя на это с аналогичной точки зрения: если вам нужно жестко указать, какое значение будет, это легко сделать с помощью любого из них. Однако, если вам нужно установить, скажем, пользовательский ввод, если вы можете просто передать значение напрямую, это сохранит шаг.
Фонд Моника иск
@ 17of26 как конкретный контрпример (чтобы показать, что «это зависит», а не тот, который предпочтительнее другого), в AppKit от Apple есть -[NSView setNeedsDisplay:]метод, который вы передаете, YESесли представление должно перерисовываться, а NOесли нет. Вам почти никогда не нужно говорить, что нет, поэтому UIKit просто не -[UIView setNeedsDisplay]имеет параметров. У него нет соответствующего -setDoesNotNeedDisplayметода.
Грэм Ли
2
@GrahamLee, я думаю, что аргумент Фаулера довольно тонкий и действительно основывается на суждении. Я бы не использовал bool isPremiumфлаг в его примере, но я бы использовал enum ( BookingType bookingType) для параметризации того же метода, если только логика для каждого бронирования не была совершенно другой. «Запутанная логика», на которую ссылается Фаулер, часто желательна, если кто-то хочет увидеть разницу между этими двумя режимами. И если они радикально отличаются, я бы раскрыл параметризованный метод извне и реализовал бы отдельные методы внутри.
Стив
3

Я думаю, что вы смешиваете две вещи в своем посте: API и реализацию. В обоих случаях я не думаю, что есть строгое правило, которое вы можете использовать постоянно, но вы должны рассмотреть эти две вещи независимо (насколько это возможно).

Давайте начнем с API, оба:

public void setHint(boolean isHintOn)

а также:

public void setHintOn()
public void setHintOff()

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

setHint(true)
// Do your process
…
setHint(false)

в то время как второй вариант дает вам это:

setHintOn()
// Do your process
…
setHintOff()

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

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

Затем вы должны выбрать способ реализации вашего публичного API. Допустим, мы выбрали методы setHintOnи setHintOffнаш публичный API, и они используют эту общую логику, как в вашем примере. Вы можете легко абстрагировать эту логику с помощью частного метода (код, скопированный из Doc):

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

И наоборот, допустим, что мы выбрали setHint(boolean isHintOn)наш API, но давайте перевернем ваш пример, по любой причине установка подсказки On полностью отличается от установки его в Off. В этом случае мы могли бы реализовать это следующим образом:

public void setHint(boolean isHintOn){
    if(isHintOn){
        // Set it On
    } else {
        // Set it Off
    }    
}

Или даже:

public void setHint(boolean isHintOn){    
    if(isHintOn){
        setHintOn()
    } else {
        setHintOff()
   }    
}

private void setHintOn(){
   // Set it On
}

private void setHintOff(){
   // Set it Off 
}

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

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

jesm00
источник
3
В качестве примечания: если это что-то вроде псевдоблока (один оператор в начале, один в конце), вы, вероятно, должны использовать beginи endили синонимы, просто чтобы четко дать понять, что они делают, и подразумевать, что начало должно иметь конец и наоборот.
Фонд Моника иск
Я согласен с Nic и добавил бы: если вы хотите, чтобы начало и конец всегда были связаны, вы должны также предоставить языковые идиомы для этого: RAII / scope guards в C ++, usingблоки в C #, withменеджеры контекста операторов в Python, передача тело как лямбда или вызываемый объект (например, синтаксис блока Ruby) и т. д.
Даниэль Приден
Я согласен с обоими пунктами, я просто пытался привести простой пример, иллюстрирующий другой случай (хотя, как вы оба указали, это плохой пример).
jesm00
2

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

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

bool isBackgroundVisible() {
    return isHintVisible;
}    

bool isContentVisible() {
    return !isHintVisible;
}

(Я позволил себе дать реальным названиям слоев - если у вас нет этого в исходном коде, я бы начал с этого)

Это все еще оставляет вас с вопросом о том, иметь ли setHintVisibility(bool)метод. Лично я бы рекомендовал заменить его с showHint()и hideHint()методом - как будет на самом деле просто и вам не придется менять их , когда вы добавляете слои. Однако, это не однозначно правильно / неправильно.

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

doubleYou
источник
Итак, tl; dr: Вы рекомендуете разделить на две функции, потому что вам не придется менять их при добавлении слоев? Если добавлен layer4, нам также может понадобиться сказать «this.layer4.visibility = isHintOn», поэтому я не уверен, что согласен. Во всяком случае, это недостаток, так как теперь, когда слой добавлен, мы должны редактировать две функции, а не только одну.
Эрдрик Айронроуз
Нет, я (слабо) рекомендую это для ясности ( showHintпротив setHintVisibility). Я упомянул новый слой только потому, что OP беспокоился об этом. Кроме того , нам нужно просто добавить еще один новый метод: isLayer4Visible. showHintи hideHintпросто установите для isHintVisibleатрибута значение true / false, и это не изменится.
дваждыВы
1
@doubleYou, вы говорите, что более длинный код не менее автоматически обслуживается. Я бы сказал, что длина кода является одной из основных переменных в удобстве обслуживания и ясности, пересекаемой только структурной сложностью. Любой код, который становится длиннее и структурно сложнее, должен его заслужить, в противном случае простые проблемы получают более сложную обработку в коде, чем они того заслуживают, а кодовая база получает заросли ненужных строк (проблема «слишком много уровней косвенности»).
Стив
@ Стив, я полностью согласен с тем, что вы можете перерабатывать вещи, то есть делать код длиннее, не делая его более понятным - о, вы всегда можете сделать код короче за счет ясности, поэтому здесь нет отношения 1: 1.
doubleYou
@Steve Подумайте «код гольф» - принятие такого кода и переписывания его в большем количестве линий делает обычно делает его более четким. «Кодовый гольф» - это крайность, но все еще есть много программистов, которые думают, что объединить все в одно умное выражение «элегантно» и, возможно, даже быстрее, потому что компиляторы недостаточно хорошо оптимизируют.
Блэкджек,
1

Булевы параметры хороши во втором примере. Как вы уже поняли, логические параметры сами по себе не являются проблематичными. Это переключение поведения на основе флага, что проблематично.

Первый пример, тем не менее, проблематичен, потому что наименование указывает метод установки, но реализации кажутся чем-то другим. Таким образом, у вас есть антипаттерн с переключением поведения и обманчиво названный метод. Но если метод на самом деле является обычным установщиком (без переключения поведения), то с этим проблем нет setState(boolean). Наличие двух методов, setStateTrue()и setStateFalse()это просто без необходимости усложнять вещи без пользы.

JacquesB
источник
1

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

Например, в Java вы можете сделать:

public enum HintState {
    SHOW_HINT(true, false, true),
    HIDE_HINT(false, true, false);

    private HintState(boolean layer1Visible, boolean layer2Visible, boolean layer3Visible) {
         // constructor body and accessors omitted for clarity
    }
}

И тогда ваш код вызывающего абонента будет выглядеть так:

setHint(HintState.SHOW_HINT);

И ваш код реализации будет выглядеть так:

public void setHint(HintState hint) {
    this.layer1Visible = hint.isLayer1Visible();
    this.layer2Visible = hint.isLayer2Visible();
    this.layer3Visible = hint.isLayer3Visible();
}

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

Даниэль Приден
источник
0

Поэтому у меня вопрос: если булев параметр используется только для определения значений, но не для поведения (например, нет в этом параметре if-else), рекомендуется ли по-прежнему исключать этот булев параметр?

Когда у меня есть сомнения по поводу таких вещей. Мне нравится представлять, как будет выглядеть трассировка стека.

В течение многих лет я работал над проектом PHP, который использовал ту же функцию, что и setter и getter . Если вы передали значение null, оно вернет значение, в противном случае установите его. С ним было ужасно работать .

Вот пример трассировки стека:

function visible() : line 440
function parent() : line 398
function mode() : line 384
function run() : line 5

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

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

function bar() : line 92
function setVisible() : line 120
function foo() : line 492
function setVisible() : line 120
function run() : line 5

Это сбивает с толку, если вы спросите меня. Одни и те же setVisibleлинии дают два разных пути трассировки.

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

Вот несколько советов:

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

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

Reactgular
источник
1
Что меня смущает в setVisibleтрассировке стека, так это то, что вызов, по- setVisible(true)видимому, приводит к вызову setVisible(false)(или наоборот, в зависимости от того, как вы получили эту трассировку).
Дэвид К
0

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

Если вы делаете не что иное, как использование состояния Enum, представляющего состояние, вы улучшили понимание своего кода.

В этом примере используется Nodeкласс из JavaFX:

public enum Visiblity
{
    SHOW, HIDE

    public boolean toggleVisibility(@Nonnull final Node node) {
        node.setVisible(!node.isVisible());
    }
}

всегда лучше, чем, как найдено на многих JavaFXобъектах:

public void setVisiblity(final boolean flag);

но я думаю , .setVisible()и .setHidden()являются лучшим решением для ситуации , когда флаг являетсяboolean поскольку он является наиболее явным и наименее многословным.

в случае чего-то с множественным выбором это еще более важно сделать таким образом. EnumSetсуществует только по этой причине.

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


источник
0

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

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

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

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

Эрик Эйдт
источник
Что, если вы разрабатываете интегрированную систему, и в конечном итоге вы являетесь одновременно производителем кода и «потребляющим клиентом» кода? Как человеку, стоящему на месте потребляющего клиента, сформулировать свое предпочтение одному подходу перед другим?
Стив
@Steve, как клиент-потребитель, вы знаете, передаете ли вы константу или переменную. Если передаются константы, предпочитайте перегрузки без параметра.
Эрик Эйдт
Но мне интересно сформулировать, почему так должно быть. Почему бы не использовать перечисления для ограниченного числа констант, поскольку это упрощенный синтаксис в большинстве языков, предназначенных именно для такого рода целей?
Стив
@ Стив, если мы знаем во время разработки / компиляции, что клиенты будут использовать постоянное значение (true / false) во всех случаях, то это говорит о том, что на самом деле есть два разных конкретных метода, а не один обобщенный метод (который принимает параметр). Я бы поспорил против введения общности параметризованного метода, когда он не используется - это аргумент YAGNI.
Эрик Эйдт
0

Есть два соображения для проектирования:

  • API: какой интерфейс вы представляете пользователю,
  • Реализация: ясность, ремонтопригодность и т.д ...

Их не следует смешивать.

Это прекрасно, чтобы:

  • иметь несколько методов в делегате API для одной реализации,
  • иметь один метод в диспетчеризации API для нескольких реализаций в зависимости от условия.

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


На стороне API

Как программист, я обычно предпочитаю программируемые API. Код гораздо понятнее, когда я могу переслать значение, чем когда мне нужно выражение if/ switchдля значения, чтобы решить, какую функцию вызывать.

Последнее может быть необходимо, если каждая функция ожидает разные аргументы.

В вашем случае, таким образом, единственный метод setState(type value) кажется лучше.

Тем не менее , нет ничего хуже, чем безымянные true, falseи 2т. Д. ... эти магические ценности не имеют значения сами по себе. Избегайте примитивных навязчивых идей и придерживайтесь строгой типизации.

Таким образом, с API POV, я хочу: setState(State state).


На стороне реализации

Я рекомендую делать все, что проще.

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


Наконец, рассмотрим группировку .

В вашем примере (с добавленными пробелами для удобства чтения):

this.layer1.visible = isHintOn;
this.layer2.visible = ! isHintOn;
this.layer3.visible = isHintOn;

Почему layer2противодействует тенденция? Это особенность или это ошибка?

Возможно ли иметь 2 списка [layer1, layer3]и [layer2], с явным именем, указывающим причину, по которой они сгруппированы, а затем выполнить итерацию по этим спискам.

Например:

for (auto layer : this.mainLayers) { // layer2
    layer.visible = ! isHintOn;
}
for (auto layer : this.hintLayers) { // layer1 and layer3
    layer.visible = isHintOn;
}

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

Матье М.
источник
0

Отдельно от вопроса setOn() + setOff()против set(flag), я бы тщательно подумал, лучше ли здесь использовать логический тип. Вы уверены, что никогда не будет третьего варианта?

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

setHint(false)

против

setHint(Visibility::HIDE)

С помощью enum будет намного легче расширяться, когда кто-то решит, что он хочет использовать опцию «при необходимости»:

enum class Visibility {
  SHOW,
  HIDE,
  IF_NEEDED // New
}

против

setHint(false)
setHint(true)
setHintAutomaticMode(true) // New
Фил
источник
0

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

Я бы предложил переоценить эти знания.

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

В вашем примере вы оцениваете параметр прямо в вашем методе. В этом отношении он ничем не отличается от других параметров.

В общем, нет ничего плохого в использовании логических параметров; и, очевидно, какой-либо параметр будет определять поведение, или с чего бы вам это вообще понадобилось?

Anoe
источник
0

Определение вопроса

Ваш титульный вопрос: "Это неправильно [...]?" - но что вы имеете в виду под «неправильно»?

Согласно компилятору C # или Java, это не так. Я уверен, что вы знаете об этом, и это не то, что вы спрашиваете. Боюсь, кроме этого, у нас только nпрограммисты n+1разные мнения. Этот ответ представляет то, что книга Чистый Кодекс говорит об этом.

Ответ

Чистый код делает веские аргументы против аргументов функции в целом:

Аргументы жесткие. Они берут много концептуальной силы. [...] наши читатели должны были бы интерпретировать это каждый раз, когда они видели это.

«Читатель» здесь может быть потребителем API. Это также может быть следующий кодировщик, который еще не знает, что делает этот код, - которым вы могли бы стать через месяц. Они будут либо проходить через 2 функции отдельно, либо через 1 функцию дважды , один раз помня trueи один раз falseпомня.
Короче говоря, используйте как можно меньше аргументов .

Конкретный случай аргумента флага позже рассматривается напрямую:

Аргументы флагов ужасны. Передача логического выражения в функцию - это действительно ужасная практика. Это сразу усложняет сигнатуру метода, громко заявляя, что эта функция делает больше, чем одно. Он делает одно, если флаг имеет значение true, и другое, если флаг имеет значение false!

Чтобы ответить на ваши вопросы напрямую: в
соответствии с «Чистым кодом» рекомендуется исключить этот параметр.


Дополнительная информация:

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


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

if (isAfterSunset) light.TurnOn();
else light.TurnOff();

Я действительно согласен , что - то неоптимальным, что здесь происходит. Может быть, это слишком очевидно, чтобы увидеть, но ваше самое первое предложение упоминает «важность избегать [sic] использования булевых параметров для определения поведения», и это является основой для всего вопроса. Я не вижу причины, чтобы сделать эту вещь, которую плохо делать проще для пользователя API.


Я не знаю, проводите ли вы тестирование - в этом случае также подумайте об этом:

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

Р. Шмитц
источник
Вы действительно похоронили Леду здесь: «... в вашем первом предложении упоминается« важность избегать [sic] использования логических параметров для определения поведения », и это является основой для всего вопроса. Я не вижу причина сделать эту вещь, которую плохо делать проще для пользователя API. " Это интересный момент, но вы скорее подрываете свой собственный аргумент в последнем абзаце.
Wildcard
Похоронен на Леде? Суть этого ответа - «использовать как можно меньше аргументов», что объясняется в первой половине этого ответа. Все, что после этого - просто дополнительная информация: опровержение противоречивого аргумента (другого пользователя, а не OP) и что-то, что не относится ко всем.
Р. Шмитц
Последний абзац просто пытается прояснить, что заглавный вопрос недостаточно четко определен, чтобы на него можно было ответить. ОП спрашивает, если это «неправильно», но не говорит, в зависимости от того, кто или что. По словам компилятора? Похоже, правильный код, так что не так. Согласно книге «Чистый код»? Он использует аргумент флага, так что да, это «неправильно». Однако вместо этого я пишу «рекомендуется», потому что прагматично> догматично. Как вы думаете, мне нужно сделать это более ясным?
Р. Шмитц
так подождите, вы защищаете свой ответ, что заглавный вопрос слишком неясен, чтобы на него можно было ответить? : D Хорошо ... Я действительно думал, что цитата, которую я процитировал, была интересным новым взятием.
Wildcard
1
Теперь ясно ясно; отличная работа!
Wildcard