LINUX.ORG.RU

Чем плохо много return?

 ,


0

3

В линтерах во многих языках программирования есть такая проверка: https://docs.astral.sh/ruff/rules/too-many-return-statements/

По ссылке приведён тривиальный пример, где достаточно сопоставления по ключу.

В общем случае код выглядит примерно как:

def f(x):
  if cond1(x):
    return res1
  y = process1(x);
  if cond2(y):
    return res2
  if cond3(y):
    return res3
  z = process2(x, y)
  if cond4(x,y,z):
    return res4
  if cond5(x,y,z):
    return res5
  ...  

после убирания return получается что-то вроде

def f(x):
  done = False 
  if cond1(x):
    res = res1
    done = True
  if not done:
    y = process1(x);
  if not done and cond2(y):
    res = res2
    done = True
  if not done and cond3(y):
    res = res3
    done = True
  if not done:
    z = process2(x, y)
  if not done and cond4(x,y,z):
    res = res4
    done = True
  if not done and cond5(x,y,z):
    res = res5
    done = True
  ...
  return res  

Второй вариант действительно легче читается или я неправильно преобразовал первый во второй?

UPD: С учётом наличия elif

def f(x):
  done = False 
  if cond1(x):
    res = res1
    done = True
  if not done:
    y = process1(x);
    if cond2(y):
      res = res2
      done = True
    elif cond3(y):
      res = res3
      done = True
  if not done:
    z = process2(x, y)
    if cond4(x,y,z):
      res = res4
      done = True
    elif cond5(x,y,z):
      res = res5
      done = True
  ...
  return res  

Вот это читается легче, чем первый вариант?

★★★★★

Последнее исправление: monk (всего исправлений: 1)

Чем плохо много return?

Документация линтера буквально говорит:

Why is this bad?

Functions or methods with many return statements are harder to understand and maintain, and often indicative of complex logic.

Очень странная предьява, но опять же - это питон, тут принято принимать странные правила на верочку.

Bfgeshka ★★★★★
()

Второй вариант тяжелее читается. Первый вариант - это классический запуск функции с последующей проверкой результата.

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

По крайней мере, в си это так. Но ты привёл код на пайтоне и зачем-то прикрепил тег «си». Не уверен, каким боком это тут.

u5er ★★★
()
Последнее исправление: u5er (всего исправлений: 1)

По ссылке приведён тривиальный пример,

очевидно, речь там идёт про частные случаи

madcore ★★★★★
()
Ответ на: комментарий от Bfgeshka

Часто бывает два стула - много return или goto, выбирай на свой вкус как говориться.

aiqu6Ait ★★★★
()

Второй вариант действительно легче читается

Нет, первый читается легче.

CrX ★★★★★
()
Ответ на: комментарий от Bfgeshka

Очень странная предьява, но опять же - это питон, тут принято принимать странные правила на верочку.

Видел такие и про Си и про Яву.

monk ★★★★★
() автор топика
Ответ на: комментарий от Bfgeshka

Очень странная предьява, но опять же - это питон, тут принято принимать странные правила на верочку.

Там дальше приводится пример и как исправить. И в таком случае действительно оно имеет смысл. В сабже же не тупо связки условие-значение, а есть дополнительная логика, так что под данный антипаттерн он не попадает. Ну а линтеры — это просто тупые проверялки часто встречающихся штук. Не стоит на них молиться. Запустил, глянул на что ругается, в данном конкретном случае сказал «а, ну это не оно», продолжил работать.

CrX ★★★★★
()
Ответ на: комментарий от u5er

По крайней мере, в си это так. Но ты привёл код на пайтоне и зачем-то прикрепил тег «си». Не уверен, каким боком это тут.

Вот на Си такая же проверка:

https://github.com/kokke/tiny-lint-c/blob/master/src/check_return_count_in_functions.c

/* Emit a warning if there are more than 3 return-statements in a function */
#define MAX_RETURNS_PR_FUNC 3

monk ★★★★★
() автор топика
Ответ на: комментарий от madcore

очевидно, речь там идёт про частные случаи

Линтер ругается всегда. В некоторых организациях требуют перед push ублажить линтер.

monk ★★★★★
() автор топика
Ответ на: комментарий от monk

Основное, что следует осознать: линтер — не догмат и не священное писание. Это инструмент. Он показывает, на что обратить внимание, подсвечивает, так сказать, потенциальные точки для рефакторинга. Но это не значит, что на каждое его замечание стоит реагировать. Если бы было именно куча if’ов с return’ами, где по сути просто от какого-то проверяемого значения зависит возвращаемое — тогда да, идеоматичнее было бы переписать в виде словаря. В примере не оно — значит просто игнорируем мнение линтера по этому поводу и идём дальше.

CrX ★★★★★
()
Последнее исправление: CrX (всего исправлений: 1)
Ответ на: комментарий от monk

И для чего она нужна? Ограничения ради ограничений?

По ссылке из шапки приведён аргумент, что когда много return, то код читается тяжелее, но это 4.2. Если больше аргументов нет, то просто выкинь это из головы.

u5er ★★★
()

В общем случае код выглядит примерно как:

def f(x):
 if cond1(x):
   return res1
 y = process1(x);
 if cond2(y):
   return res2
 if cond3(y):
   return res3
 z = process2(x, y)
 if cond4(x,y,z):
   return res4
 if cond5(x,y,z):
   return res5
 ...  

после убирания return получается что-то вроде

def f(x):
 if cond1(x):
   res = res1
 else:
   y = process1(x);
   if cond2(y):
     res = res2
   elif cond3(y):
     res = res3
   else:
     z = process2(x, y)
     if cond4(x,y,z):
       res = res4
     elif cond5(x,y,z):
      res = res5
 ...
 return res  

FTFY

Второе читается проще. И наглядно показывает проблему алгоритма. Ты что-то не так делаешь, если у тебя много уровней вложений «если». Ранний возврат в данном случае - накрытие мусора газеткой.

mogwai ★★★★★
()
Последнее исправление: mogwai (всего исправлений: 1)

один вход - один выход. Тогда проще отлаживать/оптимизировать/поддерживать. Не догмат, но принцип

MKuznetsov ★★★★★
()
Ответ на: комментарий от CrX

Там дальше приводится пример и как исправить. И в таком случае действительно оно имеет смысл.

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

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

Bfgeshka ★★★★★
()
Ответ на: комментарий от mogwai

Ты что-то не так делаешь, если у тебя много уровней вложений «если».

Как сделать это по-другому?

Условно, надо проверить входные данные (cond1) открыть какой-то ресурс, проверить возможные ошибки (cond2, cond3), что-то с ним сделать, проверить результат (cond4, cond5).

monk ★★★★★
() автор топика
Ответ на: комментарий от mogwai

Второе читается проще. И наглядно показывает проблему алгоритма. Ты что-то не так делаешь, если у тебя много уровней вложений «если». Ранний возврат в данном случае - накрытие мусора газеткой.

Ты ему сам пририсовал «много уровней вложений» там, где были вполне нормальные последовательные проверки.

Bfgeshka ★★★★★
()

если ты добавишь какой-нибудь условный malloc/free, то в первом варианте тебе придется добавить этот free перед каждым return.

В классическом Си это решают с помощью трюка с goto (пишут вместо return goto cleanup, и после cleanup в конце функции очищают все ресурсы), но если эти malloc еще и зависят каждый от своего condX, то получается вообще северный пушной зверь и никакие goto не помогут.

Lrrr ★★★★★
()

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

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

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

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

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

В том же Ruff для Python есть, например, идиотское правило RUF012. Если его принудительно не выключить, все, как макаки на каждом атрибуте дожны писать ClassVar. Идиотский карго-культ.

В общем, не стоит искать особого смысла в каждом правиле линтера. Многие из них стоит выключить, но теперь в команде идёт не борьба за то, как писать код, а за то, какие правила полезны, а какие - нет.

Chiffchaff
()
Ответ на: комментарий от MKuznetsov

один вход - один выход. Тогда проще отлаживать/оптимизировать/поддерживать. Не догмат, но принцип

Разве вторую в этой теме проще отлаживать/оптимизировать/поддерживать? Или её надо было как-то по-другому писать?

monk ★★★★★
() автор топика

прежде обсуждения идеологем вшитых в линтер

вариант с done=True - псевдоструктурно(програмирная) фекаля

если уж преседать за ради единичного return ( типо «единственность входа и выхода функции» то есть два очевидных подхода

через break из нулевого do-while <- но это тоже нарушение ультра-структурно-прогр эпигонов <- что авторам реального структурного прога только бальзам

тебе тут done ваще излишен можно res как сигнальный использовать

пока rec изначальный продолжаем проверять новые предикаты

ваще у тя по сути cond лиспика

как это оформить в python

ну например any(zip(предикаты,респонсы))

:)

qulinxao3 ★☆
()
Ответ на: комментарий от Lrrr

но если эти malloc еще и зависят каждый от своего condX, то получается вообще северный пушной зверь и никакие goto не помогут.

Делается RAII вручную с несколькими метками cleanup.

И даже с free мне кажется, что читать проще, чем проверять, что при done = true точно больше ничего до конца функции не выполнилось.

monk ★★★★★
() автор топика

Подозреваю, что конкретно в случае из ОП особой разницы нет. Но вот если кроме всех этих ветвлений есть общая часть, которую надо сделать в финале функции (не в начале и не в середине) – там вариант с res и одним return-ом в конце выглядит логичнее. А так бывает довольно часто.

// По Питону не спец, но раз ты ещё и тег c нарисовал, я тоже вставил 5 копеек.

hobbit ★★★★★
()
Последнее исправление: hobbit (всего исправлений: 1)
Ответ на: комментарий от monk

вторая это механизация Бёма - Якопини что всякий код можно на основе ветвлений и циклов и блоков приправив достаточным количеством флагов для последующий верных ветвей

само по себе это не достаточное что бы отказываться от уместных «ограниченых» goto

qulinxao3 ★☆
()
Ответ на: комментарий от qulinxao3

тебе тут done ваще излишен можно res как сигнальный использовать

Тогда нужен невозможный результат. Он не всегда есть.

ваще у тя по сути cond лиспика

Действия между условиями же. В лиспике также будет или много return или лесенка вложенных условий.

monk ★★★★★
() автор топика
Ответ на: комментарий от qulinxao3

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

«Заставь дурака Богу молиться, он весь лоб расшибёт».

hobbit ★★★★★
()
Последнее исправление: hobbit (всего исправлений: 1)

на самом деле что первый, что второй вариант откровенно так себе, основная проблема в том, что названия методов мало о чем говорят, а новые вводные появляются по мере выполнения тела метода - выглядит как типичный код, который пишут математики, которые в дальнейшем эту нетленку сопровождать не собираются. Если бы мы перешли на что-то более-менее приближенное к непосредственно логике программы (какие-то «бизнес-идеи»), у нас вырисовалось бы еще 2-3 вспомогательных метода и код нормально бы читался вне зависимости от наличия или отсутствия return early

borisych ★★★★★
()
Последнее исправление: borisych (всего исправлений: 1)
Ответ на: комментарий от hobbit

но раз ты ещё и тег c нарисовал, я тоже вставил 5 копеек.

Для этого и нарисовал.

есть общая часть, которую надо сделать в финале функции

Её можно вынести в отдельную функцию.

Или можно заменить в первом варианте return на goto end. Линтер будет ругаться ещё больше, но точно ли это менее читаемо/сопровождаемо?

monk ★★★★★
() автор топика

def f(x):
  res=object() 
  def act1():
     ...
 
  def actN():
     ...
   
  any(zip(listOfPredicates,listOfActions))
  return res
  
qulinxao3 ★☆
()
Ответ на: комментарий от Psilocybe

эээээ, а elseif нетуть чтоли?

Если в двух местах if на elif поменять, что-то изменится? Даже строк столько же останется.

monk ★★★★★
() автор топика
Ответ на: комментарий от monk

ну не нужно тут вложенности

достаточно «цикла Дейкстры» :)

ойы не знает в смысле Ткачёва перевода Оберона Вирта

повторный блок с набором защит и реакций

в данном случае это однократный «цикл» с детерминированным очерёдностью провалом до срабатывающего предиката и выдачи (break али return не суть

ваще можно и yeild :)

qulinxao3 ★☆
()
Последнее исправление: qulinxao3 (всего исправлений: 1)

типа

def f(x):
  if cond1(x):
    res=res1
  else 
    y = process1(x);
    if cond2(y):
      res=res2
    else if cond3(y):
      res=res3
    else
      z = process2(x, y)
      if cond4(x,y,z):
         res=res4
      elseif cond5(x,y,z):
        res=res5
...
   return res
Psilocybe ★★★★★
()
Ответ на: комментарий от monk

Делается RAII вручную с несколькими метками cleanup.

ну я и говорю, северный пушной зверь. Вставил malloc в середину - и все, иди ищи к какой метке он относится. Вставил еще один return - и все метки поехали, если например ты их называешь по номерам. А если не по номерам - то как проверить, что они в правильном порядке? Любой шаг в сторону - UB и расстрел на месте.

Нормальный код так не пишут.

Lrrr ★★★★★
()
Ответ на: комментарий от monk

но точно ли это менее читаемо/сопровождаемо?

На мой взгляд – менее, логика теряется.

Уместное применение goto – это выход из вложенного цикла в тех языках, где нет break с меткой. Вот в Java есть (в плюсы, говорят, в последние стандарты тоже подвезли, но по работе моя практика ограничена дай бог C++11 и может быть, кое-что из 14-го). Больше я даже не упомню, для чего сейчас goto может быть уместен без превращения кода в лапшу.

hobbit ★★★★★
()

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

Эта точка зрения была прописана в ряде известных стандартов кодирования, в том числе, например, MISRA C.

Для C это имеет смысл например для того, чтобы проще было освобождать все ресурсы в одном месте.

Для современных языков с конструкциями типа try-finally, я считаю, это всё стало неактуально.

vbr ★★★★★
()
Ответ на: комментарий от monk

да, возвраты неочевидны, если тело большое. А если точка выхода одна, то ты на это надеешся

Psilocybe ★★★★★
()
Ответ на: комментарий от Bfgeshka

Смысл поменялся? Или ты предпочтёшь написать

if not done checkA:
  done = True
  doA()
if not done and checkB
  done = True
  doB()
if not done and checkC
  done = True
  doC()

там, где можно написать

if not done:
  if checkA:
    doA()
  elif checkB:
    doB()
  elif checkC:
    doC()

?

mogwai ★★★★★
()
Ответ на: комментарий от mogwai

Вот эти if not done это додумывания ТСа по поводу угоманивания линтера, которые отсутствуют в оригинальном коде. Вся переменная и танцы вокруг неё не нужны по сути, так что смысл поменялся, да.

Bfgeshka ★★★★★
()
Ответ на: комментарий от Lrrr

А если не по номерам - то как проверить, что они в правильном порядке?

Поэтому метку нужно называть именами функции, которыми нужно выполнить очистку. Например, для malloc() я использую метку l_free, для initFoo() я использую имя метки l_deinitFoo и т.д.. Тогда в конце функции за «успешным» return выстраивается чёткая и понятная цепочка из меток с очистками для каждого этапа.

Разумеется, это сработает только в том случае, если код оформлен правильно: есть отступы и комментарии. Если идёт сплошная стена кода, то от этого мало толку.

u5er ★★★
()
Ответ на: комментарий от mogwai

Поправил начальный текст.

monk ★★★★★
() автор топика
Ответ на: комментарий от monk

Разве вторую в этой теме проще отлаживать/оптимизировать/поддерживать? Или её надо было как-то по-другому писать?

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

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

MKuznetsov ★★★★★
()
Ответ на: комментарий от Psilocybe

return можно рассматривать как разновидность goto, а Дейкстра с ней боролся

Нельзя. Иначе можно сказать, что if, for, while, break, continue это тоже разновидности goto.

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

vbr ★★★★★
()
Последнее исправление: vbr (всего исправлений: 1)
Ответ на: комментарий от Lrrr

Вставил malloc в середину - и все, иди ищи к какой метке он относится.

Вообще, free на пустой указатель выполнять можно всегда. Если malloc не запускался, в указателе NULL (при инициализации ставить).

monk ★★★★★
() автор топика
Ответ на: комментарий от monk

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

У тебя в ОП описан не этот алгоритм.

mogwai ★★★★★
()
Ответ на: комментарий от MKuznetsov

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

А как?

monk ★★★★★
() автор топика
Ответ на: комментарий от Bfgeshka
def f(x):
 if cond1(x):
   return res1
 y = process1(x);
 if cond2(y):
   return res2
 if cond3(y):
   return res3
 z = process2(x, y)
 if cond4(x,y,z):
   return res4
 if cond5(x,y,z):
   return res5

которые отсутствуют в оригинальном коде

Сработает идентично тому (с т.з. «какие операции выполнятся, какие пропустятся»), что он нагородил с

done=True
if not done
if not done
if not done
if not done

Так что нет.

mogwai ★★★★★
()
Для того чтобы оставить комментарий войдите или зарегистрируйтесь.