2009-08-25 3 views
4

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

Вот две функции (гораздо больше функций этого рода в моем коде):

def fooA(param1, param2): 
    if param2 == True: 
     code_chunk_1 

    fooA_code #uses only param1 

    if param2 == True: 
     code_chunk_2 


def fooB(param1, param2): 
    if param2 == True: 
     code_chunk_1 

    fooB_code #uses only param1 

    if param2 == True: 
     code_chunk_2 

Моя первая идея состояла в том, чтобы использовать этот декоратор:

def refactorMe(func): 
    def wrapper(*args): 
     if args[-1]: 
      code_chunk_1 

     func(*args) 

     if args[-1]: 
      code_chunk_2 

    return wrapper 

И наконец:

@refactorMe 
def fooA(param1, param2): 
    fooA_code #uses only param1 

@refactorMe 
def fooB(param1, param2): 
    fooB_code #uses only param1 

К сожалению, я не доволен этим решением:

  • Этот декоратор «навязчивый» и специфический для fooB функций fooA &
  • param2 не используется больше в fooA & fooB теле, но мы должны держать его в подписи функции

Возможно Я не использую декоратор для своей первоначальной цели?

Есть ли другой способ рефакторинга кода?

Большое спасибо!

+0

Извините, я не понимаю вашу проблему. Decorator предлагает вам код рефакторинга и исключает дублированный код. Param2 можно отправить в декоратор или быть частью функции. Таким образом, вы знаете, что принимает функция. Кроме того, используйте док-строки для разработки использования. – iElectric

+0

Что случилось с «специфичным для функций fooA & fooB»? Можете ли вы объяснить, почему это плохо? –

+0

Потому что у меня есть утверждение «args [-1]» в декораторе. Означает, что украшенная функция всегда должна иметь последний логический параметр и не может использоваться в общих случаях. – Thorfin

ответ

3

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

Вы могли бы сделать то, что говорит Нед (хотя, я бы использовать functools.partial вместо определения fooA/fooB): обычное письмо

import functools 

... 

fooA = functools.partial(call_one, _fooA) 
fooB = functools.partial(call_one, _fooB) 

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

Вы можете переписать декоратор, чтобы функции, которые только принимают один параметр, но вернуть функции, которые принимают два:

def refactorMe(func): 
    def wrapper(parm1, parm2): 
     if parm1: 
      code_chunk_1 

     func(parm1) 

     if parm2[-1]: 
      code_chunk_2 

    return wrapper 

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

В итоге:

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

Я хотел бы принять во внимание тот факт, что этот код специфичен для ваших функций fooXXX, сделав внутренний декоратор и требуя от него необходимого количества необходимых аргументов (поскольку подписи foo (* args, ** kwargs) делают самоанализ болью).

def _refactorMe(func): 
     @functools.wraps(func) #the wraps decorator propagates name/docsting 
     def wrapper(parm1, parm2): 
      if parm1: 
       code_chunk_1 

      func(parm1, parm2) 

      if parm2: 
       code_chunk_2 

     return wrapper 

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

@_refactorMe 
def fooB(param1, param2): 
    fooB_code #uses only param1 


@_refactorMe 
def fooB(param1, param2): 
    fooB_code #uses only param1 
+0

Спасибо Аарон за резюме и подробные объяснения! – Thorfin

+0

К сожалению, вы предполагаете, что все fooXXX имеют всегда 2 аргумента, что не всегда так. – Thorfin

6

Как насчет:

def call_one(func, param1, param2): 
    if param2: 
     code_chunk_1 

    func(param1) 

    if param2: 
     code_chunk_2 

def _fooA(param1): 
    fooA_code #uses only param1 

def _fooB(param1): 
    fooB_code #uses only param1 

def fooA(param1, param2): 
    call_one(_fooA, param1, param2) 

def fooB(param1, param2): 
    call_one(_fooB, param1, param2) 
+0

Я подозреваю, что code_chunk_1, code_chunk_2. fooA_code и fooB_code используют общие переменные (в противном случае это было бы так же просто, как создание def chunk1() и т. д.); в этом случае предлагаемое решение не будет работать из-за разных местных жителей. –

+1

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

+0

Спасибо, Нед, ты прав, нет общих переменных. Я уже рассмотрел это решение, и в этом случае мне пришлось бы создать две функции (fooA & _fooA). С декоратором меньше кода (я хочу реорганизовать его :)), так что это более читаемо ИМХО. – Thorfin

5

Поскольку вы пытаетесь включить некоторые функции обертку тогда и только тогда передается параметр Правда, следует использовать именованные аргументы. Вот реальный пример, который будет обернуть свой код в (database-) сделки по запросу:

def wrap_transaction(func): 
    def wrapper(*args, **kwargs): 
     # If the option "use_transaction" is given, wrap the function in 
     # a transaction. Note that pop() will remove the parameter so 
     # that it won't get passed to the wrapped function, that does not need 
     # to know about its existance. 
     use_transaction = kwargs.pop('use_transaction', False) 

     if use_transaction: 
      get_connection().begin_transaction() 

     try: 
      result = func(*args, **kwargs) 
     except: 
      if use_transaction: 
       get_connection().rollback() 
      raise 

     if use_transaction: 
      get_connection().commit() 

     return result 

    return wrapper 

@wrap_transaction 
def my_func(param): 
    # Note that this function knows nothing about the 'use_transaction' parameter 
    get_connection().exec("...") 


# Usage: Explicitely enabling the transaction. 
my_func(param, use_transaction=True) 
+0

Отличный пример в реальном мире! –

+0

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

+0

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

1

Я хотел бы сделать простой метод экстракт рефакторинга:

def _code_chunk_1(param): 
    if param == True: 
     code_chunk_1 

def _code_chunk_2(param): 
    if param == True: 
     code_chunk_2 

def fooA(param1, param2): 
    _code_chunk_1(param2) 

    fooA_code #uses only param1 

    _code_chunk_2(param2) 

def fooB(param1, param2): 
    _code_chunk_1(param2) 

    fooB_code #uses only param1 

    _code_chunk_2(param2) 

Декоратор выглядит неуместно меня в этом контексте. Ответ Неда выше также выглядит красиво.

0

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

class debugging: 
    def __init__(self, show): 
     self.show = show 

    def __call__(self, f): 
     def wrapper(*args): 
      if self.show: 
       print "inside", f 

      rv = f(*args) 

      if self.show: 
       print "outside", f 

      return rv 

     return wrapper 

@debugging(True) 
def test(n): 
    print n 

test(10) 

напечатает

inside <function test at 0x7fb28ff102a8> 
10 
outside <function test at 0x7fb28ff102a8> 
+0

Проблема с этим решением заключается в том, что параметр «показать» не оценивается во время выполнения. Я хочу называть fooA («stuff», True) или fooA («stuff», False) где угодно Хочу – Thorfin

1

Мне нравится ответ Фердинанда Байера, и я думаю, что нам нужны примеры, чтобы понять, о чем мы говорим. Я просто собираюсь дать еще два вдохновляющих предложения.

Почему явным образом не использую код транзакции?

def fooA(param1, use_transaction=param2): 
    enter_transaction(param2) 
    fooA_code #uses only param1 
    exit_transaction(param2) 

def fooB(param1, use_transaction=param2): 
    enter_transaction(param2) 
    fooB_code #uses only param1 
    exit_transaction(param2) 

Теперь, что написано, мы понимаем, что мы, вероятно, следует написать так:

def fooA(param1, use_transaction=param2): 
    with transaction(param2): 
     fooA_code #uses only param1 

def fooB(param1, use_transaction=param2): 
    with transaction(param2): 
     fooB_code #uses only param1 

Используя некоторый контекст менеджера.

Но подождите! Мы можем положить это на улицу!

если вы хотите это использовать:

with transactional(): 
    fooA(param1) 

not param2 для случая, просто вызовите fooA(param1)

Последний синтаксис предложение, когда param2 == верно:

do_transaction(fooA, param1) 

здесь мы определяем

def do_transaction(func, *args): 
    code_1 
    func(*args) 
    code_2 

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

+0

Большое вам спасибо за то, что вы познакомили меня с заявлением! Но я хотел бы сохранить простоту одной функции с параметром для «операции ввода/выхода» Последняя часть (do_transaction) похожа на идею Ned – Thorfin

0

Имея подобную проблему, я придумал более общим решение, которое позволяет:

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

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

Пример:

@decorator_more_args_prepend 
def add_params_a_b(f, a, b, *args, **kw): 
    """ 
    Wrapper whose description we don't want in the generated function 

    Line that we want in the generated function 

    :param a: description of a 
    :param b: description of b 

    No idea about the rest of the arguments 
    """ 
    print("Doing something with %s and %s, calling %s with args %s" % (a, b, f.__name__, args)) 
    return f(*args, **kw) 

@add_params_a_b 
def test_func(c, d): 
    """ 
    Test function that we want augmented 

    :param c: description of c 
    :param d: description of d 
    """ 
    print("Called with c=%(c)s and d=%(d)s" % locals()) 

Помощь:

Help on function test_func in module __main__: 

test_func(a, b, c, d) 
    Test function that we want augmented 

    Line that we want in the generated function 

    :param a: description of a 
    :param b: description of b 

    No idea about the rest of the arguments 

    :param c: description of c 
    :param d: description of d 

Вызов:

>>> test_func(1, 2, 3, 4) 
Doing something with 1 and 2, calling test_func with args (3, 4) 
Called with c=3 and d=4 

Так реализация decorator_more_args_prepend использует decorator:

#!/usr/bin/env python 
# encoding: utf-8 
# A few decorators 

__author__ = "Jérôme Carretero <[email protected]>" 
__contact__ = "http://gitorious.org/py_decorators" 
__license__ = "Python" 
__credits__ = ["Michele Simionato"] 
__version__ = "1.0.0" 

from decorator import FunctionMaker, partial, inspect, decorator 

# 
def decorator_more_args_prepend(caller, func=None): 
    """ 
    Decorator that construcs a function which calls the caller on func, 
    adding the arguments of caller as first arguments of the function. 

    Directly inspired by the decorator module code, but: 

    - we build a generated signature instead of passing the callee function 
    - we generate a docstring consisting of a merging of info from wrapper and wrapped ones 

    Based on decorator.decorator, Copyright (c) 2005-2011, Michele Simionato 
    """ 
    if func is not None: # returns a decorated function 
     evaldict = func.__globals__.copy() 
     evaldict.update({'_call_': caller, '_func_': func}) 
     caller_spec = inspect.getargspec(caller) 
     callee_spec = inspect.getargspec(func) 
     def cleaned_docstring(o): 
      import pydoc 
      return pydoc.getdoc(o).split("\n") 
     caller_doc = cleaned_docstring(caller) 
     callee_doc = cleaned_docstring(func) 
     newdoc = "\n".join(callee_doc[:1] + caller_doc[1:] + callee_doc[1:]) 
     return FunctionMaker.create(
     "%s(%s)" % (func.__name__, ", ".join(caller_spec.args[1:]+callee_spec.args)), 
     "return _call_(_func_, %(shortsignature)s)", 
     evaldict, undecorated=func, __wrapped__=func, doc=newdoc, 
     ) 
    else: 
     if isinstance(caller, partial): 
      return partial(decorator, caller) 
     # otherwise assume caller is a function 
     first = inspect.getargspec(caller)[0][0] 
     evaldict = caller.__globals__.copy() 
     evaldict['_call_'] = caller 
     evaldict['decorator'] = decorator 
     return FunctionMaker.create(
     '%s(%s)' % (caller.__name__, first), 
     'return %s(_call_, %s)' % (inspect.stack()[0][3], first), 
     evaldict, undecorated=caller, __wrapped__=caller, 
     doc=caller.__doc__, module=caller.__module__) 

Редактировать: Я поместил код на gitorious и не поддерживаю его здесь.

+0

Спасибо, что сохранили поток! Любой новый комментарий всегда приветствуется. – Thorfin

Смежные вопросы