On Thu, 04 Jan 2018 17:18:35 +0000 Andrew Williams <a...@andywilliams.me> said:

> Hi,
> 
> I don't think I can do this any longer. Any comment I make about usability
> is met with
> 
> "it's easy to understand if you know the internal state/functioning of the
> object".

believe what you want... i described it in a single if statement. it requires
no knowledge of internal state. it requires knowing just this:

if (parent) { parent now responsible for unreffing }
else { you (the caller) are responsible for unreffing when done }

requires nothing more than that. why do you claim you need to know internal
state? the above if has nothing to do with internal state. it has TOTALLy to do
with externally provided params. it's the same logic as (assuming there is an
efl_child_add that has a parent param that must be non-null, and efl_add now
drops parent param as it's assumed to be NULL):

if (used efl_child_add) { parent now responsible for unreffing }
else (used efl_add) { you (the caller) are responsible for unreffing when done }

both are conditions/data provided by the caller. one is a choice by parameter
and the other is a choice by function call name.

> My premise is still
> 
> "with efl_add sometimes returning ownership and other times not this is
> confusing for the developer"

totally based on input provided by the caller. examples:

fopen("filename", "r");
vs
fopen("filename", "w");

with one i can only call fread() on the result anxd the other i can only call
fwrite(). by your logic this is highly confusing to developers and libc should
have:

fopen_read("filename");
+
fopen_write("filename");

because developers would otherwise be confused.

> I wanted for us to present something that is clear and simple without
> needing to know internal state or reading the EFL source code.
> It seems I will not succeed.

my argument is that more constructors make for something LESS simple. while it
removes the parameter you do not like, it means you have to choose which
constructor to use based on situation and know about both.

also i argue that 95%+ of uses will be with pent not allowed to be NULL.
efl.net is reliant on a loop to drive it. all of the ui is too. the loop and
its related constructs are too. ecore_audio is too. well it is reliant on the
main loop to drive it but it may not actually technically rely on parent object
(yet, but it should). eio is reliant on parent objects too. ledbus too. 

in fact a quick look at out current efl interfaces means EVERY SINGLE object we
have requires a parent EXCEPT the loop object ... and that is created for you
(and will be in future with threads and their loops). so in fact using a
non-NULL parent is in fact invalid in 100% of efl use cases. this is why i keep
pushing back strongly. your argument for confusion is actually covering 0 use
cases (and the only reason objects can be created with NULL parents now is that
we aren't policing or enforcing parents at this point and we SHOULD be. we
SHOULD have a specific "@noparent" tag for classes that indicate an object is
allowed to have no parent - it'll be by far the exception and not the rule...
by exception i mean maybe 1 or 2% of all objects). does this incredibly rare
case justify multiplying the number of constructors we have and making a longer
winded one (efl_child_add) which will be actually 99% of use cases?

i think we're at a position where we see 2 totally different things. i see an
efl api where parent is basically always used.  you don't. you believe it'll be
"random" or basically sometimes NULL, sometimes not.

as i already mentioned in another mail. we're missing the policing of this. we
should actually have eo require a non-null parent for essentially every object
we have with some exceptions (like loop objects... which will be created for
you buy efl itself anyway - but in this case the check, when it finally exists,
should be disabled).

at this point in time, it'd be bad to put in a forced non-null check because
we're still mis-using efl's api a lot and need to clean this up so it's a very
small number of instances to fix before enforcing. we could add ERR logs to
non-null parents and add a lot of noise at this point... but IMHO this is what
we should do.

now what do we do if you, by accident, pass in a null parent on add? as long as
it's not one of the "i allow null parents" object types (which frankly will be
a handful in the end), we probably should not even begin construction and error
log and return NULL as the object as it's invalid. but first we need the
ability to flag objects as allowing null parents and then begin cleaning up our
code to match.

> Andy
> 
> On Thu, 4 Jan 2018 at 06:32 Carsten Haitzler <ras...@rasterman.com> wrote:
> 
> > On Wed, 03 Jan 2018 09:43:16 +0000 Andrew Williams <a...@andywilliams.me>
> > said:
> >
> > > Hi Cedric,
> > >
> > > I think I agree, if we can tidy the definitions then everything should
> > get
> > > clearer.
> > >
> > > efl_add_ref being needed for bindings implies that our definition of
> > > efl_add was not clean enough in the first place.
> >
> > we were very clear on that. for bindings it's needed (or if you want to
> > write
> > code in that style) but it's highly inconvenient and we've been over that.
> >
> > > If efl_del is "hide and unparent" then maybe what we really need is
> > > efl_gfx_hide and efl_parent_unset - but I don't see why we need to
> > unparent
> > > anyhow...
> >
> > well if it's the last ref... the unparenting cleanly removes from the
> > parent
> > AND removes the last ref as a result.
> >
> > BUT i kind of agree with you here. the unparenting is proving to be a major
> > pain in the rear. by that i mean the unparenting happens and then parent is
> > NULL and THEN as a result of this reference goes to 0 and destructors get
> > called. when the destructors are called, the parent is already NULL and
> > this
> > has proven an "odd" case i've been dealing with the efl loop work...
> > because on
> > destruction the object doesn't know what loop it belonged to anymore...
> > yes -
> > you have to handle parent_set to NULL then but it means this has to do some
> > kind of destruction of loop bound resources... :(
> >
> > IMHO when the destructors are called the object should still have a parent
> > and
> > be removed from the child list as a very last "after last destructor is
> > called"
> > step, not "before destructors are called".
> >
> > also we need to do better parent policing.
> >
> > 1. objects that can never have a NULL parent need to be marked as such
> > 1.2 objects that MUST have a loop parent at the top of their parent tree or
> > somewhere in it (provider_find for a loop class must fund a non-null loop
> > object).
> > 1.3 objects MUST have a loop object and it MUST be the main loop object and
> > only that one.
> > 2. some objects are intended to be toplevels (with NULL as the parent) and
> > should be marked as such (e.g. loop objects).
> >
> > > If we are delegating to a parent to manage the lifecycle of the object
> > then
> > > we should step away from the reference and forget it - that is the most
> > > "convenient" behaviour I guess.
> > >
> > > if:
> > > efl_add *always* returned an owned reference and took no parent
> > > efl_add_child *never* returned an owned reference and required a parent
> > >
> > > then:
> > > efl_add_ref* would no longer be required right? (if the binding requires
> > a
> > > ref after efl_add_child we have efl_ref that it could wrap up)
> > > efl_del would take a reference and dec (probably not needed as we have
> > > efl_unref?)
> > > efl_del_child seems unlikely to be needed as all that is left is hiding
> > > which is a graphics call, not an eo lifecycle one.
> > >
> > > The more I look at it the more I think we have too much UI related
> > thinking
> > > in our object lifecycle.
> >
> > that's because 90% of our objects have been UI related in the past sand
> > pretty
> > much still are. for us, i think this is the right thing.
> >
> > > Andy
> > >
> > > On Wed, 3 Jan 2018 at 05:41 Cedric Bail <ced...@ddlm.me> wrote:
> > >
> > > > > The whole efl_del argument just exist because it is kinda poorly
> > > > > named. IMO, del means: get this object to an "empty" state.
> > > > > Just like close to files and hide and unparent to UI objects. efl_del
> > > > > should not steal references under people who owns it, the object
> > > > > would get deleted at a later time when everybody using the object
> > > > > stops doing so, we could even return errors from efl_del'eted
> > > > > objects for methods that do not make sense anymore, causing
> > > > > most actions to, possibly, halt earlier rather than later.
> > > >
> > > > So, if efl_del does not still a references under people who owns it,
> > how
> > > > do we fix it ? Should it still magically reset its parent to NULL when
> > > > there is one and just efl_unref in the other case ? Should it be
> > symetric
> > > > to efl_add_ref and always reset the parent to NULL along with unref ?
> > Or
> > > > should it do none of this at all and you have to manually do the
> > parent set
> > > > and the unref ?
> > > >
> > > > Trying to figure out what behavior would make it work for binding, I
> > would
> > > > guess it would be best to just make it symetric to efl_add_ref. This
> > will
> > > > give a predictable outcome I think, but I am not sure it is enough.
> > What do
> > > > you think ?
> > > >
> > > > >   IMO, the whole problem with efl_add/efl_add_ref is that
> > > > > "parents" are treated specially, which they should not. parent_set
> > > > > should increment efl_ref and parent_unset should decrement it.
> > > >
> > > > Agreed and surprised it is not the case.
> > > >
> > > > > For C, OTH, where we do expect some "automatism" on resource
> > > > > handling, efl_unref'ing may be too much of a hassle when a
> > > > > parent is already going to handle the lifetime of the object. So,
> > > > > it would make sense, IMO, for efl_add_scope. It could even be
> > > > > that efl_add_scope is named efl_add, no problem, as long as
> > > > > there's a efl_add that keeps this semantics for binding
> > > > > development. Which also means that parent_set/unset must
> > > > > be fixed.
> > > >
> > > > I think that once efl_del behavior is clearly defined, the existence
> > of an
> > > > efl_add_scope/efl_add will also be clearer to everyone.
> > > >
> > > > >   Also, @own tags must not relate to parent_set, because that
> > > > > has no useful information for tags or users, if needed we can
> > > > > add a @reparent tag, but that's not really special information
> > > > > such as real owernship information.
> > > >
> > > > I am still wondering what the @own really mean. Does that mean that the
> > > > object own at least one reference of it ? But in that case, doesn't
> > that
> > > > mean that the user need to always ref it, if it plans to keep it
> > around.
> > > >
> > > > As for @reparent, I am not sure we have case yet where we return an
> > object
> > > > that can not be reparented, do we have such a case ?
> > > >
> > > > Cedric
> > > >
> > > >
> > ------------------------------------------------------------------------------
> > > > 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
> > > >
> > >
> > >
> > > --
> > > 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


------------------------------------------------------------------------------
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

Reply via email to