On Tue, 26 Dec 2017 12:19:30 +0000 Andrew Williams <[email protected]> said:
> Hi, > > With the proposal of efl_add and efl_add_child we remove the need for > efl_add_ref* as the result of the former becomes consistent in its return > of owned or not owned references. no. it's needed for bindings (c++, js, lua etc.) because in c++ with raii an unref is called on exiting scope, and lua.js will do so with a gc that will discovered the orphaned scope and unref. > Hopefully Cedric can confirm this as I don’t know the spec. > Right now we have a second one because the first is not consistently > returning pointers that either do or do not need to be unrefd. then efl_add_child is not consistent with efl_add. this is just as confusing as parent being NULL or not. but now we need 4 "add" constructors because we still need efl_add_ref as above. > Andy > On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler <[email protected]> wrote: > > > On Sun, 24 Dec 2017 09:16:08 +0000 Andrew Williams <[email protected]> > > said: > > > > > Are you trolling me now? > > > > no. you said its inconsistent. it's consistent. it has a simple rule. > > > > http://www.dictionary.com/browse/consistent > > > > it consistently adheres to the same principles. i described them. > > > > > A method that does two different things depending on some magic value / > > > > it's not a MAGIC value. it's a parent object handle. it's far from magic. > > it's > > "put this object into THIS box here, or into NO box and give it to me" > > based on > > that option. > > > > > null flag is a code smell (see Clean Coders if this is not familiar). > > > Consider this method: > > > > > > ptr get_mem(string poolname, long bytes) { > > > If (poolname == NULL) > > > return malloc(bytes); // MUST be freed > > > else > > > return get_pool(poolname).borrow(bytes); // must NOT be freed > > > } > > > > > > Do you think that is consistent? The user is not sure without inspecting > > > the parameter contents whether or not the should free(). This is > > > conceptually what we are setting up. > > > > #define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__) > > > > happy? you can have a macro to hide the parent if NULL. but it'll be used > > fairly rarely. > > > > > Back to our efl_add - what would be consistent is this: > > > > > > Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent) > > > > > > Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must > > not > > > be null, should not be unrefd > > > > > > That is consistent. It is also compliant with the V7 vote. It still has > > the > > > race condition but is much easier to read. I know from the method names > > > what is expected. > > > > your proposal was to have efl_add return void. the above is better for > > sure. i > > see we were walking down similar paths. > > > > i still dislike the above because it just makes the api more verbose for > > the > > sake of special-casing "parent == NULL". i dislike it. this isn't a magic > > "bool" that turns on or off behaviour. that is actually not great. you > > read code > > like > > > > func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE); > > > > ... and what are the 3 bools? it's not clear or obvious. but with a > > constructor > > like efl_add firstly it's incredibly common so it's something that will be > > learned very quickly, and the typing already tells you it's an object > > handle. > > and you should have learned already that it's the parent object (or no > > parent > > at all if NULL). > > > > why do i dislike it? we now go from 2 constructors (efl_add and > > efl_add_ref) to > > 4 (efl_add, efl_child_add, efl_add_ref, efl_child_add_ref). i dislike this > > "explosion" just to hide the parent arg being NULL. > > > > > Thoughts? > > > On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler <[email protected]> > > wrote: > > > > > > > On Sat, 23 Dec 2017 11:30:58 +0000 Andrew Williams < > > [email protected]> > > > > said: > > > > > > > > > Hi, > > > > > > > > > > As this thread seems to be descending into word games and (insert > > > > > appropriate word) contests I will reiterate my concern: > > > > > > > > > > efl_add is inconsistent and that should be addressed. > > > > > > > > do it's not. i explained already that it is not. i'll repeat again. > > it's > > > > consistent: > > > > > > > > if parent == valid object, then ref is owned by parent > > > > else ref is owned by caller/scope. > > > > > > > > that is consistent. > > > > > > > > > I hope that is clear enough > > > > > Andy > > > > > > > > > > > > > > > On Thu, 21 Dec 2017 at 13:15, Andrew Williams <[email protected]> > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > This is now well documented ( > > > > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) > > but > > > > the > > > > > > more I use efl_add the more I feel it is confusing especially to > > new > > > > > > developers. > > > > > > > > > > > > In the current model (if I understand it correctly) > > > > > > 1) child = efl_add(klass, parent) means the child must NOT be > > > > unfeferenced > > > > > > 2) child = efl_add(klass, NULL) means the child should be > > unreferenced > > > > > > 3) child = efl_add_ref(klass, parent) means the child must be > > > > unreferenced > > > > > > 4) child = efl_add_ref(klass, NULL) somehow means that the child > > > > should be > > > > > > unreferenced twice > > > > > > > > > > > > In my opinion 1) and 4) are peculiar and so I provide a proposal > > to fix > > > > > > this: > > > > > > > > > > > > We could change efl_add to return void. It never retains a > > reference. > > > > If > > > > > > the parent is NULL then it should be automatically unref before > > > > returning. > > > > > > Then efl_add_ref would be changed to mirror this and always retain > > > > exactly > > > > > > 1 reference - so if parent is NULL there is no double count > > returned. > > > > > > > > > > > > Using this model if an Eo * is returned then I know I have a > > reference > > > > and > > > > > > must later unref. > > > > > > Any need for using the pointer in an efl_add (which is no longer > > > > returned) > > > > > > would still be supported through efl_added within the constructor. > > > > > > > > > > > > What do people think about this? I put the suggestion forward to > > > > improve > > > > > > the symettry with add and unref which is currently confusing. If my > > > > > > assumptions above are incorrect please let me know! > > > > > > > > > > > > Thanks, > > > > > > Andy > > > > > > -- > > > > > > http://andywilliams.me > > > > > > http://ajwillia.ms > > > > > > > > > > > -- > > > > > http://andywilliams.me > > > > > http://ajwillia.ms > > > > > > > > > > > ------------------------------------------------------------------------------ > > > > > Check out the vibrant tech community on one of the world's most > > > > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > > > > _______________________________________________ > > > > > enlightenment-devel mailing list > > > > > [email protected] > > > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > > > > > > > > > -- > > > > ------------- Codito, ergo sum - "I code, therefore I am" > > -------------- > > > > Carsten Haitzler - [email protected] > > > > > > > > -- > > > http://andywilliams.me > > > http://ajwillia.ms > > > > > > -- > > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > > Carsten Haitzler - [email protected] > > > > -- > http://andywilliams.me > http://ajwillia.ms -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- Carsten Haitzler - [email protected] ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
