Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How Awful is My Decorator?

Tags:

python

I recently created a @sequenceable decorator, that can be applied to any function that takes one argument, and causes it to automatically be applicable to any sequence. This is the code (Python 2.5):

def sequenceable(func):
    def newfunc(arg):
        if hasattr(arg, '__iter__'):
            if isinstance(arg, dict):
                return dict((k, func(v)) for k, v in arg.iteritems())
            else:
                return map(func, arg)
        else:
            return func(arg)
    return newfunc

In use:

@sequenceable
def unixtime(dt):
    return int(dt.strftime('%s'))

>>> unixtime(datetime.now())
1291318284
>>> unixtime([datetime.now(), datetime(2010, 12, 3)])
[1291318291, 1291352400]
>>> unixtime({'start': datetime.now(), 'end': datetime(2010, 12, 3)})
{'start': 1291318301, 'end': 1291352400}

My questions are:

  • Is this a terrible idea, and why?
  • Is this a possibly good idea, but has significant drawbacks as implemented?
  • What are the potential pitfalls of using this code?
  • Is there a builtin or library that already does what I'm doing?
like image 649
Brent Newey Avatar asked Dec 22 '22 20:12

Brent Newey


2 Answers

This is a terrible idea. This is essentially loose typing. Duck-typing is as far as this stuff should be taken, IMO.

Consider this:

def pluralize(f):
    def on_seq(seq):
        return [f(x) for x in seq]
    def on_dict(d):
         return dict((k, f(v)) for k, v in d.iteritems())
    f.on_dict = on_dict
    f.on_seq = on_seq
    return f

Your example then becomes

@pluralize
def unixtime(dt):
    return int(dt.strftime('%s'))


unixtime.on_seq([datetime.now(), datetime(2010, 12, 3)])
unixtime.on_dict({'start': datetime.now(), 'end': datetime(2010, 12, 3)})

Doing it this way still requires the caller to know (to within duck-typing accuracy) what is being passed in and doesn't add any typechecking overhead to the actual function. It will also work with any dict-like object whereas your original solution depends on it being an actual subclass of dict.

like image 89
aaronasterling Avatar answered Jan 09 '23 02:01

aaronasterling


In my opinion, you seem to be building logic into the wrong place. Why should unixtime know anything about sequencing? In some cases it would be a good idea (for performance or even semantics) but here it seems like you're adding extra features to unixtime that don't make sense in that context.

Better is just to use (say) a list comprehension:

[unixtime(x) for x in [datetime.now(), datetime(2010, 12, 3)]]

that way, you're using the proper Pythonic construct for applying the same thing to a sequence, and you're not polluting unixtime with ideas about sequences. You end up coupling logic (about sequencing) in places where the implementation should be free of that knowledge.

EDIT: It basically comes down to coding style, reusability and maintainability. You want well partitioned code, so that when you're coding unixtime (say), you're concerned exclusively with converting to a unixtime. Then, if you're interested in sequences, you design functionality (or use built-in syntax) that is exclusively concerned with sequences. That makes it easier to think clearly about each operation, test, debug and reuse code. Think about it even in terms of the name: the original function is appropriately called unixtime, but your sequenced version might more appropriately be called unixtime_sequence, which is weird and suggests an unusual function.

Sometimes, of course, you break that rule. If (but only when) performance is an issue, you might combine functionality. But in general, partitioning things at first into clear parts leads to clear thinking, clear coding and easy reuse.

like image 23
Peter Avatar answered Jan 09 '23 01:01

Peter