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

Reply via email to