[zeromq-dev] Proposal for a revised zlist API

[Goswin von Brederlow]

Just saw your commit.

1) Small typos: The comment for zlist_dup mentions zhash_dup_v2. The
comments for zlist_sort talk about key and value and straight ASCII
comparison. But sorting goes by the compare function.

2) zlist_purge does not destroy the cursor allowing zlist_next and
zlist_item access to freed memory.

3) change cursor to "node_t **" but that would be internal.

4) Now there is zlist_detach and zlist_delete. How about
zlist_detach_current and zlist_delete_current for the cursor based
flavour of the same? (needs 3)

5) zlist_set_comparator for a container wide compare function.

6) zlist_sort could allow NULL as second argument to use the container
compare (or compare by address, however useless that is :). (needs 5)

7) zlist_find can be added without name collision. (needs 5)

8) zlist_detach and zlist_delete need to specify their affect on the
cursor.

9) I would suggest having zlist_detach and zlist_delete use container
compare function.

10) I would suggest having zlist_detach and zlist_delete use zlist_find
+ zlist_*_current. (needs 7, solves 9 implicitly)

11) zlist_t still has a "bool autofree;". The old behaviour could be
achieved by having zlist_autofree set a duplicator (strdup) and
destructor (something that calls free) instead.

12) Can we add the "void *user" argument to czmq_destructor and
czmq_comparator? That would be realy usefull for keeping items in 2
container like zcertstore does among other things.

13) zlist_first, zlist_next and zlist_last each duplicate zlist_item
at the end.

14) zlist_insert_before could be added without name collision. (needs 3)

15) zlist_insert(_after) could be added without name collision.

16) zlist_insert_sorted could be added without name collision. (needs 3+5+7)

17) zlist_last is a problem for 4 since it updates the cursor.
Consider this stupid way to purge the list in reverse order:

    while (zlist_last (list)) zlist_delete_current (list);

Either zlist_last needs to search the whole list to set the cursor or
zlist_delete_current needs to search the whole list to set the tail.
If you are changing the cursor behaviour anyway (see 18) then maybe
we could make zlist_last not change the cursor. I don't see any use
for that and it would keep the complexity at O(1) instead of O(n).

If someone needs to process a zlist from the back like that then they
should be using a zring instead. So I think this would be safe.

18) You've changed the API, the cursor behaviour, slightly already. I
wonder if any code relies on the destructive cursor behaviour of
zlist_push, zlist_append, zlist_remove (and any others I forgot). It
is possible to have code like this:

void append_item_and_print_list (zlist_t *list, item_t *item) {
    zlist_append (list, item);
    do {
        item = zlist_next (list);
        print_item (item);
    } while (item);
}

The loop relies on the fact that zlist_append sets the cursor to NULL
and zlist_next then restarts it from the begining. This would now
break and only print the new item instead of all items.

MfG
	Goswin

On Thu, Sep 04, 2014 at 09:00:44PM +0200, Pieter Hintjens wrote:
> :-) I started, by coincidence, to make some of these changes already.
> 
> Hope to have this ready soon. Nothing dramatic, just the global callbacks.
> We can take the other changes one by one. Breaking the v2 API is not
> an option, and renaming zlist to another name isn't plausible either
> IMO. So we may rename some methods, like zlist_dup, which make broken
> assumptions (autofree, in that case).
> 
> -Pieter
> 
> On Thu, Sep 4, 2014 at 5:38 PM, Goswin von Brederlow <goswin-v-b at web.de> wrote:
> > On Thu, Sep 04, 2014 at 04:59:19PM +0200, Goswin von Brederlow wrote:
> >> Hi,
> >>
> >> I've gone over the zlist API and redesigned it according to some ideas
> >> we had last month (partly for the zring class). I tried this out on
> >> zlist to see how the ideas would fit but the idea would be to change
> >> all the classes where appropriate so they behave the same.
> >
> > In case it wasn't clear this is just an experiment to see how it could
> > work. To see if it makes more sense that way. No attempt is made to
> > preserve backward compatibility and none is ment.
> >
> > I wanted to design the cursor behaviour and global callbacks cleanly
> > without having to worry about making them work parallel to the old
> > API. It might be possible to do that though.
> >
> > MfG
> >         Goswin