On Sun, Sep 25, 2016 at 1:59 AM, Davide Andreoli <d...@gurumeditation.it> wrote: > 2016-09-25 4:37 GMT+02:00 Carsten Haitzler <ras...@rasterman.com>: >> On Sat, 24 Sep 2016 18:44:28 +0200 Davide Andreoli <d...@gurumeditation.it >> said: >> > 2016-09-24 12:59 GMT+02:00 Carsten Haitzler <ras...@rasterman.com>: >> > > On Sat, 24 Sep 2016 12:43:03 +0200 Davide Andreoli < >> d...@gurumeditation.it >> > > > >> > > said: >> > > >> > > > 2016-09-24 7:14 GMT+02:00 Carsten Haitzler <ras...@rasterman.com>:
<snip> >> > > maybe we should use a whole struct that holds cb, data and free cb. >> maybe >> > > thats >> > > a bit much? like >> > > >> > > typedef struct _Efl_Callback Efl_Callback; >> > > typedef void (*Efl_Data_Free_Cb) (void *data); >> > > >> > > struct _Efl_Callback { >> > > Efl_Event_Cb func; >> > > const void *data; >> > > Efl_Data_Free_Cb data_free; >> > > }; >> > > >> > > as now we have a "bag of data" that represents something to call, what >> > > to pas >> > > to it and how to clean up what is passed. it is a unit of data and so >> > > expressing it as a struct might make more sense, so now we >> > > >> > > Efl_Callback cb = {my_cb_func, my_cb_data, my_data_free}; >> > > efl_callback_add(obj, EVENT_TYPE, &cb); >> > > >> > > >> > I cannot see the benefit of using a struct here... it's more chars to >> > type >> > and more data >> > to store when more that one callback is used, like in efl_future_then(), >> > where >> > we have 3 callback pointers but just 1 *data and 1 data_free_cb >> >> well before it was 1 values always together. a func ptr + data ptr, now >> it's 3 >> values. together. always. doesn't it make sense to start grouping them as a >> unit? i dislike havin to pass yet another NULL all the time too. the >> number of >> times that data pointer needs freeing is rather rare (in my experience). > > I'm also not so happy to add yet another NULL to type, but I really cannot > see > how your proposal will improve things, especially for efl_future_then() :/ > > You want something like this? > f = efl_uri_set(url); > Efl_Callback cb1 = {my_done_cb, my_cb_data, my_cb_data_free} > Efl_Callback cb2 = {my_error_cb} > Efl_Callback cb3 = {my_progress_cb} > efl_future_then(f, cb1, cb2, cb3) I guess he had something in mind like what we do with EFL_CALLBACKS_ARRAY_DEFINE (We can't use static directly as this would break on windows). So it should look like : EFL_FUTURE_DEFINE_THEN(name1, done_cb, error_cb, progress_cb, data, data_free_cb); EFL_FUTURE_DEFINE_PROPAGATE_THEN(name2, done_cb, data, data_free_cb); and you would do : efl_future_then(f, name1()); efl_future_then(f, name2()); Arguably we could do the deduplication inside efl_future_then, but that does had a lookup in a hash table and will likely impact speed. This does the lookup for us at compile time, which is better, but I don't really like the API. > instead of this: > f = efl_uri_set(url); > efl_future_then(f, my_done_cb, my_error_cb, my_prog_cb, my_cb_data, > my_free_data) > > You are confusing me :) I agree with you, we are already paying the price of using Eo here, I guess we do not care that much about memory and speed anyway (Do not forget that we do have a full fledged Eo object allocated per future here, this is way more expensive than 3 pointers). So should we make our API that much worse for a gain that will be impossible to see ? If we move future to be their own object, or use an Eo light object, maybe that would make sense, but otherwise, I think we are missing the point here. We want a nice API ! > Another solution in my mind would be to leave efl_future_then as is (6 > params) and > define an efl_then() variadic macro that accept from 3 to 6 args (I'm not > sure about the exact syntax here) I am not a big fan of variadic and this would still push for doing deduplication at run time, so not a big fan and I am pretty sure it doesn't really help with any real problem. -- Cedric BAIL ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel