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)
Ответ на: комментарий от monk
if this.cond1():
    return res1
  this.process1();
  if this.cond2():
    return res2

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

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

Но из них же всё равно не собрать вменяемый монадный чейн через опшенал?

Собрать. Главное, чтобы тот, кто это сопровождать будет, не знал, где ты живёшь.

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

Я что-то туплю наверное и не понимаю затруднения, вот же

    exit_condition, result = steps[1](result)

явно прописано какие данные откуда берутся и куда уходят. Т.е. тут вообще нету пространства для интерпретации.

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

написанием лапши из глобальных переменных и безусловных прыжков

А в скандале с микроконтроллером ЕМНИП тойотовского движка разве не такая лапша фигурировала? Недодавил Дейкстра, получается.

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

явно прописано какие данные откуда берутся и куда уходят

Прописано, что берут из структуры(?) result и возвращают новую структуру. А то, что первый step будет читать поле x и никуда не писать, второй читать поле x и писать y, это только внутри шагов смотреть. Вместо

  if cond1(x):
    return res1
  y = process1(x);

получаем

  ex, res = step1(res)
  if ex: return res
  ex, res = step2(res)
  if ex: return res
monk ★★★★★
() автор топика
Ответ на: комментарий от monk

это только внутри шагов смотреть.

Ну да. Это информация должна быть где-то записана. Любо внутри шага, либо в объемлющей функции. Тут никакой магии нет. Просто в одном случае отдельно описана логика «делай шаг n, если условие, верни результат, иначе делай шаг n+q» и отдельно шаги, а в другом это всё вперемешку.

ugoday ★★★★★
()
sub f {
    $_->[1]() and return $_->[0] for (
        [ 'res 1', sub { cond1($x) } ], 
        [ 'res 2', sub { $y = process1($x); cond2($y) } ], 
        [ 'res 3', sub { cond3($y) } ], 
        [ 'res 4', sub { $z = process2($x, $y); cond4($x,$y,$z) } ], 
        [ 'res 5', sub { cond5($x,$y,$z) } ], 
        [ 'wtf?!', sub { 1 } ],
    );
}
madcore ★★★★★
()
Ответ на: комментарий от monk

Вкусовщина, конечно, но мне ближе противопоставление map, filter, reduce vs for, if. Обоими способами можно выразить одну и ту же идею, но в первом случае получается яснее, потому как каждый этап преобразования данных логически обособлен, сразу понятно тут мы наделали данных, тут отобрали нужные, тут посчитали статистику. В for, if делается то же самое, но нужно приложить усилие, чтобы понять где-чего происходит.

ugoday ★★★★★
()

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

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

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

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

Тоже не факт.

Вместо
def find(a, x):
  for s1 in a:
    for s2 in s1:
      for s3 in s2:
        if s3 == x: 
          return True
  return False

писать

def find(a, x):
  res = False
  for s1 in a:
    for s2 in s1:
      for s3 in s2:
        if s3 == x: 
          res = True
  return res

?

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

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

def f(x):
  if cond1(x):
    return res1
  return f_process1(x)

def f_process1(x):
  y = process1(x)
  if cond2(y):
    return res2
  if cond3(y):
    return res3
  return f_process2(x, y)

def f_process2(x, y):
  z = process2(x, y)
  if cond4(x,y,z):
    return res4
  if cond5(x,y,z):
    return res5
neumond ★★
()

def f(x):
    res=object()
    for _ in 1,:#while((a:=1)if'a'not in locals()else locals().__delitem__('a')):
        if cond1(x                    ):res=res1;break
        if cond2(y:=process1(x)       ):res=res2;break
        if cond3(y                    ):res=res3;break
        if cond4(x,y,z:=process2(x, y)):res=res4;break
        if cond5(x,y,z                ):res=res5;break
    return res
#иль же дажь:
def f(x):
    res=object()
    for _ in 1,:
        if 0: ...
        elif cond1(x                    ):res=res1
        elif cond2(y:=process1(x)       ):res=res2
        elif cond3(y                    ):res=res3
        elif cond4(x,y,z:=process2(x, y)):res=res4
        elif cond5(x,y,z                ):res=res5
    return res
#но тадлы:
def f(x):
    res=object()
    if 0: ...
    elif cond1(x                    ):res=res1
    elif cond2(y:=process1(x)       ):res=res2
    elif cond3(y                    ):res=res3
    elif cond4(x,y,z:=process2(x, y)):res=res4
    elif cond5(x,y,z                ):res=res5
    return res
qulinxao3 ★☆
()
Последнее исправление: qulinxao3 (всего исправлений: 4)

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

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

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

На Си «лесенку» можно не делать, а размещать каждый следующий if под else. Это не мешает понимать код, т.к. if-else-if-else-… по сути на одном логическом уровне. Меня так на работе научили делать. Еще и точки с запятой в конце блока if научили ставить там, где не предполагается наличие else. Теперь пишу в таком стиле.

void check_return_count_in_functions_new_token(struct source_file* s, struct token* toks, int tok_idx)
{
  int i = tok_idx;

  if (brace_lvl >= 1)
  {
    if (toks[i].toktyp == KW_RETURN)
       state += 1;
  }
  else
  if (brace_lvl == 0)
    state = 0;

  if (state > MAX_RETURNS_PR_FUNC)
  {
    /* one warning pr. violation is sufficient */
    if (state != state_last_warn)
    {
      state_last_warn = state;

      /* prettier names in the output: '3rd return statement in function' etc. */
      const char* endings[] = { "th", "st", "nd", "rd" };
      const char* ending = endings[0];

      if ((state < 10) || (state > 20))
      {
        if (state % 10 == 1) 
          ending = endings[1];
        else 
        if (state % 10 == 2)
          ending = endings[2];
        else
        if (state % 10 == 3) 
          ending = endings[3];
      };

      fprintf(stdout, "[%s:%d] (style) %d%s return statement in function.\n", s->file_path, toks[i - 1].lineno, state, ending);
    }; // if (state != state_last_warn)
  }; // if (state > MAX_RETURNS_PR_FUNC)

  if (toks[i].toktyp == OP_LBRACE) 
    brace_lvl += 1;
  else 
  if (toks[i].toktyp == OP_RBRACE)
    brace_lvl -= 1;

  if (brace_lvl < 0)
    brace_lvl = 0;
}
Vic
()
Последнее исправление: Vic (всего исправлений: 4)
Ответ на: комментарий от monk

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

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

если ты добавишь какой-нибудь условный malloc/free

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

seiken ★★★★★
()

Этот код нарушает Принцип Открытости/Закрытости (OCP), поскольку добавление новой логики (нового условия/процесса) требует изменения существующей функции f. OCP гласит: программные сущности (классы, модули, функции и т. д.) должны быть открыты для расширения, но закрыты для модификации.

Чтобы применить OCP, мы можем использовать шаблон Стратегия или Цепочка обязанностей. Шаблон Цепочка обязанностей подходит здесь лучше, так как у нас есть последовательность условий и действий, где каждое звено либо обрабатывает запрос, либо передает его дальше.

🛠️ Применение OCP с помощью Цепочки Обязанностей

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

1. Определяем интерфейс Обработчика (Handler)

from abc import ABC, abstractmethod
from typing import Any, Optional

# Константы для результатов (для примера)
RES1 = "Результат 1"
RES2 = "Результат 2"
RES3 = "Результат 3"
RES4 = "Результат 4"
RES5 = "Результат 5"

class Request:
    """Объект, содержащий данные для обработки и промежуточные результаты."""
    def __init__(self, x):
        self.x = x
        self.y = None  # Результат process1
        self.z = None  # Результат process2

class Handler(ABC):
    """Абстрактный класс Обработчика."""
    def __init__(self, next_handler: Optional['Handler'] = None):
        self._next_handler = next_handler

    def set_next(self, handler: 'Handler') -> 'Handler':
        """Устанавливает следующий обработчик в цепочке."""
        self._next_handler = handler
        return handler

    @abstractmethod
    def handle(self, request: Request) -> Any:
        """Обрабатывает запрос или передает его дальше."""
        if self._next_handler:
            return self._next_handler.handle(request)
        return None # Если достигнут конец цепочки без результата


2. Создаем Конкретные Обработчики

Предположим, что cond1, process1, cond2, res1 и т.д. — это некие существующие функции/значения.

# Заглушки для функций
def cond1(x): return x == 1
def process1(x): return x * 10
def cond2(y): return y < 20
def cond3(y): return y == 30
def process2(x, y): return x + y + 1
def cond4(x, y, z): return z > 100
def cond5(x, y, z): return z == 55

# --- Обработчик 1: cond1(x) и возврат res1 ---
class HandlerCond1(Handler):
    def handle(self, request: Request) -> Any:
        if cond1(request.x):
            return RES1
        return super().handle(request)

# --- Обработчик 2: process1(x) и сохранение y ---
# (Этот обработчик изменяет объект Request, но не возвращает результат, а передает дальше)
class HandlerProcess1(Handler):
    def handle(self, request: Request) -> Any:
        request.y = process1(request.x)
        print(f"DEBUG: Process1 executed, y = {request.y}")
        return super().handle(request)

# --- Обработчик 3: cond2(y) и возврат res2 ---
class HandlerCond2(Handler):
    def handle(self, request: Request) -> Any:
        if request.y is not None and cond2(request.y):
            return RES2
        return super().handle(request)

# --- Обработчик 4: cond3(y) и возврат res3 ---
class HandlerCond3(Handler):
    def handle(self, request: Request) -> Any:
        if request.y is not None and cond3(request.y):
            return RES3
        return super().handle(request)

# --- Обработчик 5: process2(x, y) и сохранение z ---
class HandlerProcess2(Handler):
    def handle(self, request: Request) -> Any:
        if request.y is not None:
            request.z = process2(request.x, request.y)
            print(f"DEBUG: Process2 executed, z = {request.z}")
        return super().handle(request)

# --- Обработчик 6: cond4(x, y, z) и возврат res4 ---
class HandlerCond4(Handler):
    def handle(self, request: Request) -> Any:
        if request.z is not None and cond4(request.x, request.y, request.z):
            return RES4
        return super().handle(request)

# --- Обработчик 7: cond5(x, y, z) и возврат res5 ---
class HandlerCond5(Handler):
    def handle(self, request: Request) -> Any:
        if request.z is not None and cond5(request.x, request.y, request.z):
            return RES5
        return super().handle(request)


3. Собираем Цепочку (Функция f закрыта для модификации)

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

def f_ocp(x):
    """Функция, использующая Цепочку Обязанностей (закрыта для модификации)."""
    
    # 1. Создаем звенья цепочки
    handler1 = HandlerCond1()
    handler2 = HandlerProcess1()
    handler3 = HandlerCond2()
    handler4 = HandlerCond3()
    handler5 = HandlerProcess2()
    handler6 = HandlerCond4()
    handler7 = HandlerCond5()

    # 2. Строим цепочку
    handler1.set_next(handler2).set_next(handler3).set_next(handler4).set_next(handler5).set_next(handler6).set_next(handler7)
    
    # 3. Создаем запрос
    request = Request(x)
    
    # 4. Запускаем обработку
    return handler1.handle(request)


🚀 Преимущества OCP

  1. Закрытость для Модификации: Если вам нужно добавить новую логику (например, cond6 и res6), вы просто создаете новый класс HandlerCond6 и расширяете цепочку в месте её сборки (f_ocp), не меняя при этом код существующих обработчиков или общей логики обработки.
  2. Открытость для Расширения: Добавление новой функциональности (нового класса Handler) очень легко.
  3. Лучшая Тестируемость: Каждый обработчик — это маленький, изолированный класс, который легко тестировать отдельно.

📝 Пример использования

print(f"f_ocp(1) -> {f_ocp(1)}")
# Output: f_ocp(1) -> Результат 1 (Сработал HandlerCond1)

print("-" * 20)
# Допустим, x=3, process1(3) = 30, что соответствует cond3
print(f"f_ocp(3) -> {f_ocp(3)}") 
# Output:
# DEBUG: Process1 executed, y = 30
# f_ocp(3) -> Результат 3 (Сработал HandlerCond3)

print("-" * 20)
# Допустим, x=10, process1(10) = 100, process2(10, 100) = 111
# 111 не удовлетворяет cond4, cond5
print(f"f_ocp(10) -> {f_ocp(10)}")
# Output:
# DEBUG: Process1 executed, y = 100
# DEBUG: Process2 executed, z = 111
# f_ocp(10) -> None (Цепочка не нашла результат)

Могу ли я показать вам, как добавить новое условие (cond6) без изменения существующих классов обработчиков?

oxo
()

В данном варианте я все-таки за первый, но с созданием пустых строк:

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
  ...  

Если функция f здоровая, я бы разбил ее на несколько функций.

dicos ★★☆
()