Skip to content

Conversation

@amstan
Copy link
Contributor

@amstan amstan commented Jan 17, 2015

This makes it easy to reuse wrapped python functions in other C code.

This is to prevent the keepalive savings, and to make the code neater looking.
Old discussions: amstan@1d1db6f

This makes it easy to reuse wrapped python functions in other C code.
@mgeier
Copy link
Member

mgeier commented Jan 18, 2015

Is it possible that the two functions you are using (set_error_function() and set_info_function()) are the only ones where returning the CFFI function pointer actually makes sense?

@amstan
Copy link
Contributor Author

amstan commented Jan 18, 2015

You're right, the set_*_functions returning cffi function pointers are only necessary when you want to pass the same functions along to other cffi stuff.

All i'm trying to do is avoid duplicating the wrapping/reference saving from _set_error_or_info_function in my code, and use it on the same python function again.

@mgeier
Copy link
Member

mgeier commented Jan 19, 2015

So what about returning the cdata only in set_error_function() and set_info_function() and leaving the rest unchanged?

@amstan
Copy link
Contributor Author

amstan commented Feb 22, 2015

Abandoning this, I changed my code to not need this, the cffi stuff could be used directly if really needed.
amstan/guitar@d55fbf8#diff-7607b77999779e616acb8412424510de

@amstan amstan closed this Feb 22, 2015
@amstan amstan deleted the cffi-returns branch February 22, 2015 04:02
@mgeier
Copy link
Member

mgeier commented Feb 22, 2015

Wow, I find the idea of using @jack.set_* (and @myclient.set_*_callback) as decorator very intriguing; I wasn't aware that these functions can be used like that!

How did you come up with this? Is this a common idiom?

I was thinking about using this idiom in the examples, but the presence of the userdata argument might be a bit confusing, which in turn led me to the idea of dumping userdata altogether: #7.

@amstan
Copy link
Contributor Author

amstan commented Mar 8, 2015

I was thinking about using this idiom in the examples, but the presence of the userdata argument might be a bit confusing, which in turn led me to the idea of dumping userdata altogether: #7.

It's a nice idea, you just need to keep in mind that using it as a decorator will eat the original function(the way stuff is right now). You would have to return the register_callback argument(the callback itself) for all of the functions that you have(similar to how i wanted to do it with the cffi versions of the callbacks).

Wow, I find the idea of using @jack.set_* (and @myclient.set_*_callback) as decorator very intriguing; I wasn't aware that these functions can be used like that!

The way i had them totally failed. You can't decorate instance methods easily since at decoration time self is not known, so it will actually register a 2 parameter function ([self, msg] instead of just [msg]).

You can use this pattern in your examples just fine, chatty_client.py should work perfectly.

  • Given that you don't need to call those callbacks manually, so you don't need to return the callback from the register function.
  • And you're not dealing with methods either

@amstan
Copy link
Contributor Author

amstan commented Mar 8, 2015

Another way that jackclient could deal with all these callback setters is just with inheritance:

class GuitarSeq(jack.Client):
    def on_process(self):
        #override whatever the default is for the client
    def on_xruns(self):
        super().on_xruns() #maybe some debugging could happen by default, similar to chatty client
        #code to reduce xruns

Designing jack_client to work this way would not necessary be backwards incompatible, the jack.Client.set_*_callback functions should still work since jack_set_*_callback functions work if they're called multiple times. The first time could be in jack.Client.__init__ to various overridable methods like jack.Client.on_process, and the second, in case of "legacy" code.

@mgeier
Copy link
Member

mgeier commented Mar 9, 2015

Thanks for the comments regarding decorators.
I think I'll use that in the examples (at least in chatty_client.py, where there are so many callbacks) once #7 is resolved.

I don't like the solution with inheritance, though.
In general, I try to avoid creating classes as much as possible.
In this case the functions set_*_callback() are very simple to use and very flexible (they accept free functions and bound methods and basically anything callable). IMHO, there is no benefit for the user in creating a class derived from jack.Client.
And if some users can't resist the urge of creating a class (or if they actually have good reasons), they'll have to type a bit more, but in return, by using the set_*_callback() functions, their code gets more explicit and more readable.

More importantly, I think there is even a quite bad disadvantage, since all callbacks are called, even if they are empty functions. At least for the process callback, this adds unnecessary overhead.

mgeier added a commit that referenced this pull request May 23, 2015
@mgeier
Copy link
Member

mgeier commented May 23, 2015

As suggested above, I changed the examples to register the callbacks with decorators, see 8f8916a.

I really like how this looks now, thanks @amstan for this nice idea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants