Лучшая практика - Обтекание, если вокруг вызова функции против добавления раннего выхода, если защита в функции

9

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

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

Обтекание, если вокруг вызова функции


if (shouldThisRun) {
  runFunction();
}

Есть ли ( охранник ) в функции

runFunction() {
  if (!shouldThisRun) return;
}

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


Вот пример

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

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

Мэтью Маллин
источник
3
@gnat Нет, этот вопрос, по сути, «Какой предпочтительный синтаксис при досрочном выходе», а мой - «Должен ли я выйти досрочно или просто не вызывать функцию»
Мэтью Маллин
4
Вы не можете доверять разработчикам, даже себе , правильно проверять предварительные условия функции везде, где она вызывается. по этой причине я бы предложил, чтобы функция проверяла внутренне любые необходимые условия, если она способна это сделать.
TZHX
«Я хочу, чтобы статус обновлялся только в том случае, если статус изменился» - вы хотите, чтобы статус обновлялся (= изменялся), если изменился тот же статус? Звучит довольно круто. Не могли бы вы уточнить, что вы подразумеваете под этим, чтобы я мог добавить содержательный пример к моему ответу по этому вопросу?
Док Браун
@DocBrown Допустим, например, что я хочу синхронизировать свойства состояния двух разных объектов. Когда любой объект изменяется, я вызываю syncStatuses () - но это может быть вызвано многими различными изменениями поля (не только поля состояния).
Мэтью Маллин

Ответы:

15

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

Защитное предложение в функции (досрочное возвращение):
это защищает от вызова с недопустимыми параметрами

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

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

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

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

Baldrickk
источник
Потрясающий ответ. Мне нравится тот факт, что это дает мне четкий способ думать об этом. If должно быть снаружи, поскольку это «часть процесса принятия решения» относительно того, должна ли функция вызываться в первую очередь. И по своей сути не имеет ничего общего с самой функцией. Мне кажется странным отмечать правильный ответ, но через несколько часов я посмотрю еще раз и сделаю это.
Мэтью Маллин
Если это помогает, я не рассматриваю это как «мнение». Я отмечаю, что это «неправильно», но это потому, что разные уровни абстракции не являются отдельными. Что я понял из твоего вопроса, так это то, что ты видишь, что это не «правильно», но, поскольку ты не думаешь об уровнях абстракции, его трудно определить количественно, поэтому ты изо всех сил пытаешься выразить это словами.
Baldrickk
7

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

bool tryRunFunction(...)
{
    bool shouldThisRun = /* some logic using data not available inside "runFunction"*/;
    if (shouldThisRun)
        runFunction();
    return shouldThisRun;
}

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

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

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

Док Браун
источник
1
Должен признаться, идиома tryXXX () всегда казалась немного странной, и здесь она неуместна. Вы не пытаетесь что-то сделать, ожидая вероятной ошибки. Вы обновляете, если он грязный.
user949300
@ user949300: выбор правильного имени или схемы именования зависит от реального варианта использования, реальных имен функций, а не от какого-то придуманного имени runFunction. Функция like updateStatus()может сопровождаться другой функцией like updateIfStatusHasChanged(). Но это зависит от случая в 100%, для этого не существует единого решения, так что да, я согласен, идиома «попробуй» не всегда хороший выбор.
Док Браун
Там нет чего-то под названием "DryRun"? Более или менее становится обычным исполнением без побочных эффектов. Как отключить побочные эффекты - это другая история
Laiv
3

Что касается того, кто решает, бежать ли, ответ от GRASP , кто является "экспертом по информации", который знает.

Как только вы решили, рассмотрите переименование функции для ясности.

Примерно так, если функция решит:

 ensureUpdated()
 updateIfDirty()

Или, если звонящий должен решить:

 writeStatus()
user949300
источник
2

Я бы хотел расширить ответ @ Baldrickk.

Там нет общего ответа на ваш вопрос. Это зависит от значения (контракта) вызываемой функции и характера условия.

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

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

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

Итак, вопросы:

  • С точки зрения извне, когда разрешено / требуется вызывать функцию, принимая во внимание контракт функции?
  • Есть ли внутри функции ситуации, когда она может вернуться рано без какой-либо реальной работы?
  • Соответствует ли условие, вызывать ли функцию / возвращать ли раньше, внутреннему домену функции или внешнему?

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

Ральф Клеберхофф
источник
2

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

if (shouldThisRun) {
   runFunction();
}

runFunction() {
   if (!shouldThisRun) return;
}

И вам нужно вызвать другую функцию в runFunctionметоде, как это:

runFunction() {
   if (!shouldThisRun) return;
   someOtherfunction();
}

Что ты бы сделал? Копируете ли вы все проверки сверху вниз?

someOtherfunction() {
   if (!shouldThisRun) return;
}

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

public someFunction() {
   if (shouldThisRun) {
      runFunction();
   }
}

private runFunction() {
 // do your business.
}
Engineert
источник