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)
Ответ на: комментарий от Kogrom

А предложенная альтернатива нарушает сразу 2 принципа - KISS и YAGNI.

самые вредные принципы )

KISS: Используется для отказа от необходимых абстракций («и так сойдет»).

YAGNI: Блокирует проектирование, приводит к архитектурному долгу, когда добавление функциональности требует переписывания.

DRY: Приводит к преждевременному обобщению и необоснованному связыванию модулей.

oxo
()
Последнее исправление: oxo (всего исправлений: 2)
Ответ на: комментарий от oxo

Вот это ты серьёзно?

В смысле, если в чужой функции обнаружишь

def f(x):
    handler1 = HandlerCond1()
    handler2 = HandlerProcess1()
    handler3 = HandlerCond2()
    handler4 = HandlerCond3()
    handler5 = HandlerProcess2()
    handler6 = HandlerCond4()
    handler7 = HandlerCond5()

    handler1.set_next(handler2).set_next(handler3).set_next(handler4).set_next(handler5).set_next(handler6).set_next(handler7)
    return handler1.handle(Request(x))

то тебе будет гораздо понятнее её алгоритм, чем если наткнёшься на текст функции из темы?

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

Разумеется (с учётом что будет нормальное именование), а не как в примере.

И я получаю бонусом огромное количество ништяков:

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

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

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

За сколько времени поймёшь, что делает функция f?

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

def f(x):
  handler1 = HandlerLess0()
  handler2 = HandlerMore0Cont()
  handler3 = HandlerEq0()
  handler1.set_next(handler2).set_next(handler3)
  return handler1.handle(Request(x))

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

class Handler:
    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 # Если достигнут конец цепочки без результата

# --- Обработчик 1: условие меньше 0 ---
class HandlerLess0(Handler):
    def handle(self, request: Request) -> Any:
        if request.x < 0:
            return 1
        return super().handle(request)

# --- Обработчик 2: условие больше 0 ---
class HandlerMore0Cont(Handler):
    def handle(self, request: Request) -> Any:
        if request.x > 0:
            request.x = request.x - 1
            z = request.z
            request.z = request.y + request.z
            request.y = z
            self.handle(request)
        return super().handle(request)

# --- Обработчик 3: условие равно 0 ---
class HandlerEq0(Handler):
    def handle(self, request: Request) -> Any:
        return request.z - 1
monk ★★★★★
() автор топика
Ответ на: комментарий от monk

Кажется у нас тут два вопроса:

  1. Из топика «В общем случае» – я это понял что например в «process» у нас не x = x - 1, а например IO операция и что мы в целом находимся в контексте большого приложения. В этом случае я прав, а ты нет
  2. Если мы говорим про то как закодить алгоритм и нам process это реально x = x -1 – ты прав, а я нет
oxo
()
Последнее исправление: oxo (всего исправлений: 1)
Ответ на: комментарий от oxo
  1. process сложный, но вызывается одной строкой. Его действие уже вынесено в отдельную функцию и у этой функции есть входящие параметры и результат, видимые в коде, в отличие от handleProcess.

  2. Цепочка мне напоминает предварительный результат компиляции Бейсика.

Там после синтаксического разбора такое и было

line10.set_next(line20).set_next(line30)
line30.set_nextif(line20, line40)
line40.set_next(line50)
line50.set_nextif(line20, line60)

Называется, если нет goto, но очень хочется, то можно такое использовать.

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

KISS: Используется для отказа от необходимых абстракций («и так сойдет»).

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

YAGNI: Блокирует проектирование, приводит к архитектурному долгу, когда добавление функциональности требует переписывания.

Проектирование нужно, когда есть сложные данные. Если у нас набор действий, то всё проектирование - разделение кода на функции. Тут даже самое простейшее ООП только усложнит код. А в предложенной альтернативе не просто иерархия, там мини-фреймворк для написания внутреннего DSL.

Кроме того, что код без всякой нужды громоздкий, он ещё и по быстродействию хуже. Та же цепочка handler1.set_next(handler2).set_next(handler3).set_next(handler4)… не может прерваться, её нужно обходить всю, если не использовать хаков типа прерываний. В исходном коде и в коде с однозвенным циклом выйти из алгоритма можно в любом месте, если встретим нужное условие.

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

Всё же можно записать цепочку не таким громоздким методом. А если бы у питона имелись менее калечные лямбды, так и вовсе красиво было бы. Но имеем что имеем.


def stepper(result, steps):
    if not steps or result["condition"] is None:
        return result
    else:
        result = steps[0](result)
        return stepper(result, steps[1:])

def w(env):
    x, y, z = env['x'], env['y'], env['z']
    while x > 0:
      x = x - 1
      y, z = z, y + z
    return z - 1

def f2(x):
    return stepper({'x': x, 'condition': True},
                   [lambda env: env.update({'x': env['x'] - 2}) or env,
                    lambda env: {'x': 1, 'condition': None} if x < 2 else env,
                    lambda env: env.update({'y': 1, 'z': 1}) or env,
                    w])

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

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

Если уж делать из питона хаскелл, то как-то так:

def f2(x):
    return stepper({'x': x, 'condition': False},
                   [lambda env: env.update({'x': env['x'] - 2}) or env,
                    lambda env: env.update({'result': 1, 'condition': env['x'] < 0}) or env,
                    lambda env: env.update({'y': 1, 'z': 1}) or env,
                    w(env) or env])

def w(env):
  ...
  env.update({'result': z-1})
monk ★★★★★
() автор топика
Последнее исправление: monk (всего исправлений: 3)
Ответ на: комментарий от monk

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

Признаться, мне не кажется это важным в данной ситуации. Можно для явного выделения параметра останова вместо словаря использовать кортеж (error, env). Те же яйца вид в профиль.

Насчёт,

В списке функций как действия, так и условия

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

P.S. Всё равно тут главная проблема — отсутствие нормальных лямбд. А она не решается, разве python4 мастерить начнут.

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

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

Во втором продолжит проверять условия после первого же done = true, не делая ничего полезного. Во-первых, гонять пустые такты - дурной тон. Во-вторых, Питон медленный и переменные в нем громоздкие. Имеем антипаттерн, который попьет крови на большой нагрузке или в цикле со множеством итераций. В-третьих, чем больше переменных, детерминирующих поведение, тем труднее читать код.

С elif - еще более трудночитаемая городуха.

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

Vidrele ★★★★★
()