Re: New GQueue API

[Owen Taylor <otaylor redhat com>]

On Thu, 2003-09-18 at 14:46, Soeren Sandmann wrote:
> In 
> 
>         http://bugzilla.gnome.org/show_bug.cgi?id=78414
> 
> Owen says he thinks it makes sense to complete GQueue to get a full
> GList/GSList equivalent API. So here is the list of GList API with my
> comments on their appropriateness for GQueue:
> 
> -=-
> 
>     GList*   g_list_append         (GList            *list,
>                                     gpointer          data);
>     GList*   g_list_prepend        (GList            *list,
>                                     gpointer          data);
> 
> These functions are already available, named a bit oddly as
> g_queue_push_{head,tail}.
>
>     GList*   g_list_insert         (GList            *list,
>                                     gpointer          data,
>                                     gint              position);
>     GList*   g_list_remove         (GList            *list,
>                                     gconstpointer     data);
>     GList*   g_list_remove_all     (GList            *list,
>                                     gconstpointer     data);
>     GList*   g_list_reverse        (GList            *list);
>     GList*   g_list_copy           (GList            *list);
>     gint     g_list_index          (GList            *list,
>                                     gconstpointer     data);
>     guint    g_list_length         (GList            *list);
>     void     g_list_foreach        (GList            *list,
>                                     GFunc             func,
>                                     gpointer          user_data);
>     gpointer g_list_nth_data       (GList            *list,
>                                     guint             n);
> 
> These make sense and have an obvious counterpart for GQueue, though
> the function g_list_nth_data() should probably be called
> g_queue_peek_nth() to be consistent with g_queue_peek_front.

Sounds fine.

> -=-
> 
>     GList*   g_list_insert_sorted  (GList            *list,
>                                     gpointer          data,
>                                     GCompareFunc      func);
> 
> I think we should have
> 
>         void g_queue_insert_sorted (GQueue *queue,
>                                     gpointer data,
>                                     GCompareDataFunc func,
>                                     gpointer sort_data);
> 
> ie. the compare function should take a user argument.
> 
>     GList*   g_list_sort           (GList            *list,
>                                     GCompareFunc      compare_func);
>     GList*   g_list_sort_with_data (GList            *list,
>                                     GCompareDataFunc  compare_func,
>                                     gpointer          user_data);
>                                
> Similarly, I think we should only have g_queue_sort() that takes a
> GCompareDataFunc.

Sounds OK to me to favor simplicity over consistency here.

>     GList*   g_list_insert_before  (GList            *list,
>                                     GList            *sibling,
>                                     gpointer          data);
>     GList*   g_list_remove_link    (GList            *list,
>                                     GList            *llink);
>     GList*   g_list_delete_link    (GList            *list,
>                                     GList            *link_);
>     GList*   g_list_nth            (GList            *list,
>                                     guint             n);
>     GList*   g_list_nth_prev       (GList            *list,
>                                     guint             n);
>     GList*   g_list_find           (GList            *list,
>                                     gconstpointer     data);
>     GList*   g_list_find_custom    (GList            *list,
>                                     gconstpointer     data,
>                                     GCompareFunc      func);
>     gint     g_list_position       (GList            *list,
>                                     GList            *llink);
>     GList*   g_list_last           (GList            *list);
>     GList*   g_list_first          (GList            *list);
> 
> These make sense if and only if we consider GList * links iterators
> into GQueue. I am not sure if that's desirable, but note that we
> already have

queue->head/tail are public, so we aren't hiding the fact that 
the links in a GQueue are GList. So, I think that things like

 void g_queue_insert_before (GQueue   *queue,
                             GList    *sibling.
                             gpointer  data);

are fine.

> The function corresponding to g_list_nth() should probably be named
> g_queue_peek_nth_link().
> 
> -=-
> 
>     GList*   g_list_concat         (GList            *list1,
>                                     GList            *list2);
> 
> This could possibly make sense as
> 
>         g_queue_concat (GQueue *queue, GQueue *other);
> 
> Should this free @other, or should the contents just be moved to
> queue?

See comments about g_queue_push_list_{head|tail} below. I don't
really think that concatenating queues destructively fits in
well with the idea of a 'queue' being an object.

> Some functions that could be useful, but don't correspond to anything
> in GList:
> 
>         GList *g_queue_pop_list (GQueue *queue)
> 
> returns the list. When this function returns @queue will be empty.

When would this be useful?

>         void g_queue_push_list_{head|tail} (GQueue *queue, GList *list);
> 
> adds a full list to the queue (taking ownership).

This might be the more useful replacement of 'concat'

>         gpointer g_queue_pop_nth (GQueue *queue, gint nth)
> 
> returns and removes the nth element of the queue.

Fits in well to me.

>         GQueue *g_queue_splice_nth (GQueue *queue, gint nth);
>         GQueue *g_queue_splice     (GQueue *queue, gpoitner data);
>         GQueue *g_queue_splice_link (GQueue *queue, GList *link_);
> 
> [more dubious, but may be useful if we have g_queue_concat() and
> g_queue_copy()]

Hmm, without people asking for them, I'd be hestitant to add these.

Regards,
					Owen