Are you trolling me now?

A method that does two different things depending on some magic value /
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.

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.

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