patrys / httmock

A mocking library for requests
Other
466 stars 57 forks source link

Make decorators work with instance methods #7

Closed AusIV closed 11 years ago

AusIV commented 11 years ago

This commit modifies the all_requests and urlmatch decorators so that their targets can be either functions or methods.

patrys commented 11 years ago

I like the idea but the *args voodoo is rather unreadable. Could you change it have two explicit subfunctions for the method and function case?

AusIV commented 11 years ago

I don't think there's really a more readable way to do what you suggest that isn't also brittle. Branching it out to separate subfunctions is easy enough, but there's not an easy way to tell from the decorator which one we should call.

When methods are declared, they're still considered functions at the point that the decorator gets applied, so we can't use inspect.ismethod(func). We could use inspect.getargspec(func) to see if the first argument is 'self', but that's relying on convention rather than real language rules.

Another option would be to take this approach: http://stackoverflow.com/questions/1288498/using-the-same-decorator-with-arguments-with-functions-and-methods

But that has its own set of voodoo using get & call, and doesn't work on stacked decorators under some circumstances.

The only remaining option I see is to require developers to indicate whether they're decorating a function or a method, which seems even less desirable.

patrys commented 11 years ago

What I suggest is to have a master function that accepts self_or_url as the first arg, inspects it and calls a specialized subfunction that has an explicit signature instead of relying on args[n]. What do you think?

AusIV commented 11 years ago

How is this? Rather than calling a specialized subfunction (which would duplicate parts of the codebase), it uses essentially the same logic as before but a bit more explicitly in terms of what each argument might be.

patrys commented 11 years ago

Looks good, merging!