Why does this decorator program produce an unexpected output?

user_6380992 0
Viewed 60

I have written the following program to provide wrappers (decorators) to the two functions price_report and sales_report. I have just assigned the wrappers to these functions (last two lines in the code below) without explicitly invoking price_report() or sales_report(). But the code produces the output shown further below. How come?

In fact, if I make explicit call to price_report(), I get the error message TypeError: 'NoneType' object is not callable.

# wrapper.py

def wrapper(report):
    def head_and_foot(report):
        print(report.__name__)
        report()
        print("End of", report.__name__, "\n\n")
    return head_and_foot(report)

def price_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    price = [500_000, 350_000, 800_000, 550_000]
    for x, y in zip(cars, price):
        print(f'{x:8s}', f'{y:8,d}')

def sales_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    units = [5000, 3000, 1000, 800]
    for x, y in zip(cars, units):
        print(f'{x:8s}', f'{y:8,d}')

sales_report = wrapper(sales_report)
price_report = wrapper(price_report)

Output of the above program (whether run inside Jupyter notebook or from the command line as python wrapper.py):

sales_report
Celerio     5,000
i10         3,000
Amaze       1,000
Figo          800
End of sales_report


price_report
Celerio   500,000
i10       350,000
Amaze     800,000
Figo      550,000
End of price_report
python-decorators python-3.x python
1 Answers

It's harder than it needs to be to see exactly what's going on with your code, because you've chosen confusing names in writing your decorator. Here's a version that does exactly the same thing as your code, with the names changed:

def head_and_foot(func):
    def wrapper(func):
        print(func.__name__)
        func()
        print("End of", func.__name__, "\n\n")
    return wrapper(func)

def price_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    price = [500_000, 350_000, 800_000, 550_000]
    for x, y in zip(cars, price):
        print(f'{x:8s}', f'{y:8,d}')

def sales_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    units = [5000, 3000, 1000, 800]
    for x, y in zip(cars, units):
        print(f'{x:8s}', f'{y:8,d}')

sales_report = head_and_foot(sales_report)
price_report = head_and_foot(price_report)

There are three changes here:

  • wrapper → head_and_foot
  • head_and_foot → wrapper
  • report → func

The function you called wrapper, which I've renamed to head_and_foot, is a decorator. What that means is that it accepts a function as an argument, and returns another function which is meant to replace the one it accepted.

Normally, the replacement function it returns is a wrapper for the original function, which means that it does the same thing the original function does, wrapped in some extra actions.

To keep all this straight, it's conventional to call the decorator by a name describing its effect (e.g. head_and_foot, call the function it accepts func, and call the wrapper it returns wrapper. That's what I've done above.

Once you've got sensible names, it's a little easier to see that you have two problems:

  1. wrapper is supposed to be a replacement for the functions being decorated, so it should have the same signature – meaning that it should take the same number and type of arguments. Your functions price_report and sales_report don't take any arguments at all (i.e. there's nothing between the parentheses () in their def statement), but wrapper takes the function it's supposed to replace as an argument, which makes no sense at all.

    That line should just be def wrapper(): to match the signature of the functions being replaced.

  2. A decorator is supposed to return a replacement function, but your decorator is calling the replacement and returning the result. Instead of return wrapper(func), you just need return wrapper.

After making both those changes, we end up with this:

def head_and_foot(func):
    def wrapper():
        print(func.__name__)
        func()
        print("End of", func.__name__, "\n\n")
    return wrapper

def price_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    price = [500_000, 350_000, 800_000, 550_000]
    for x, y in zip(cars, price):
        print(f'{x:8s}', f'{y:8,d}')

def sales_report():
    cars  = ['Celerio', 'i10', 'Amaze', 'Figo']
    units = [5000, 3000, 1000, 800]
    for x, y in zip(cars, units):
        print(f'{x:8s}', f'{y:8,d}')

sales_report = head_and_foot(sales_report)
price_report = head_and_foot(price_report)

When we run this fixed code, we don't get any unexpected output, but we do get two functions that do what we expect:

>>> price_report()
price_report
Celerio   500,000
i10       350,000
Amaze     800,000
Figo      550,000
End of price_report 


>>> sales_report()
sales_report
Celerio     5,000
i10         3,000
Amaze       1,000
Figo          800
End of sales_report
user_10149380
Related