Re: [jlbec evilplan org: Enumeration patch]



On Thu, Oct 19, 2000 at 10:38:44AM +0200, Alexander Larsson wrote:
> I like this idea. It doesn't solve all problems concerning list-returning
> behaviour though. It is still necessary in some cases to know if the
> enumerator is a static copy of the container state when it was enumerated,
> or if it is using the live state and is invalidated whenever the container
> is modified. 

	I would lean towards making it always invalidated.  I don't like
the caller knowing static/live state (part of the point of the
enumeration), and the caller should be able to know when the state
changes.  Anyone reading something should know to lock against writes,
etc.  The usual stuff.  So in a single-threaded app, if the user makes a
modifying call, the enumeration is invalidated.  If a multi-threaded
app, the caller should do the usual shared-read/single-write locking
around a piece of data.

> I like it's opaqueness and consistance in usage, it will make it easier
> for the user of containers, and it will make it easier to do binary
> compatible changes to the internals of thc container.

	Right, this is another thing I like about it.  You can even go
from an internal GList to an internal GWhatever, and the caller never
knows.

> Why are the g_enumeration_list_* functions in the public API? I can't see
> any reason for that.

	They started that way just like g_str_hash and g_str_equal are
public, but I've come to the same conclusion as you, that they don't
really belong in the public API.

> If the GEnumerationFunc got a pointer to the gpointer context the list
> enumerator (and other simple enumerators) won't have to allocate a special
> context struct (a list-item in the list-enumerator case). I think that
> would make the code simpler.
> 
> typedef gpointer (*GEnumerationFunc)	(gpointer *context);

	Yes, I thought about that.  Part of me really doesn't want even
the enumeration creator mucking with the actual GEnumeration structure.
I was sort of thinking of the context pointer as opaque.  You can't
change it after creation.  So while it adds that extra step to the
simple list enumerator, it protects against misuse in other situations.
Most folks would probably use _new_from_list() and never see that.
Anyone requiring a more complex enumerator will probably pass in their
own structure, which will never change address.  eg:

GtkContainer *foo;

g_enumeration_new(foo, has_more, get_next, destroy_notify);

has_more(gpointer context)
{
    GtkWidget *foo = GTK_WIDGET(context);

    /* access only parts of the foo structure here */

    return retval;
}

	I'm not totally set on this idea.  So if others agreed with you
on the ability to change the context pointer itself, I would consider
making the change.

> If, in the list-enumerator, you return the current element and then
> increment the enumerator you wouldn't have to prepend an empty element to
> the list.

	Right, but then at the end of the list I've got a NULL pointer
and no reference back to the list to g_list_free() it upon
destroy_notify.
	Latest patch, with the _list_* functions now static in
genumeration.c, is attached.

Joel

-- 

"The nearest approach to immortality on Earth is a government
 bureau."
	- James F. Byrnes

			http://www.jlbec.org/
			jlbec evilplan org

Attachment: genumeration.diff.2.gz
Description: Binary data



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]