In the books I have read they consider null being a special case of behaviour within a method just as confusing as passing a bool like in your illustration.
Andy On Tue, 26 Dec 2017 at 12:19, Andrew Williams <a...@andywilliams.me> wrote: > 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. > 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. > > Andy > On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler <ras...@rasterman.com> > wrote: > >> On Sun, 24 Dec 2017 09:16:08 +0000 Andrew Williams <a...@andywilliams.me> >> 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 <ras...@rasterman.com> >> wrote: >> > >> > > On Sat, 23 Dec 2017 11:30:58 +0000 Andrew Williams < >> a...@andywilliams.me> >> > > 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 <a...@andywilliams.me >> > >> > > 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 >> > > > enlightenment-devel@lists.sourceforge.net >> > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> > > > >> > > >> > > >> > > -- >> > > ------------- Codito, ergo sum - "I code, therefore I am" >> -------------- >> > > Carsten Haitzler - ras...@rasterman.com >> > > >> > > -- >> > http://andywilliams.me >> > http://ajwillia.ms >> >> >> -- >> ------------- Codito, ergo sum - "I code, therefore I am" -------------- >> Carsten Haitzler - ras...@rasterman.com >> >> -- > 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 enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
