stream/iterator map context nodes (Re: [redland-dev] [patch] librdf_storage_hashes_context_serialise_get_statement)

[Dave Beckett]

Lauri Aalto wrote:
> On Tue, May 6, 2008 at 5:35 PM, John Fieber <jfieber at adobe.com> wrote:
>>  That said, I do have a different problem with iterator/stream map functions
>> regarding contexts.  The mapping function for an iterator has the signature
>> (rdf_iterator.h):
>>
>>   typedef void* (*librdf_iterator_map_handler)(librdf_iterator *iterator,
>> void *map_context, void *item);
>>
>>  The problem is the list of iterator methods you cannot call within the map
>> function because they produce infinite recursion:
> 
>>  Clearly not all of those are necessary or even appropriate to use in a map
>> callback since you are handed a pointer to the object, but getting the
>> context is relevant, and the only way to get it is to violate encapsulation
>> of the iterator.  The stream methods exhibit the same behavior.
> 
> I'm thinking of one way to make context nodes accessible in map
> functions without breaking much existing code. Introduce another
> callback, say:
> 
>   typedef void*
> (*librdf_iterator_map_with_context_handler)(librdf_iterator *iterator,
> void *map_context, void *item, void *item_context);
>   // (or librdf_node *item_context)
> 
> and the API function for adding them:
> 
>   int librdf_iterator_add_map_with_context(librdf_iterator* iterator,
> librdf_iterator_map_with_context_handler map_function,
> librdf_iterator_map_free_context_handler free_context, void
> *map_context);
> 
> This way the changes can be contained in the rdf_iterator class.
> Neither iterator callback implementations nor iterator clients need to
> be changed.
> 
> (Ditto for streams. Also maybe should try to come up with better
> typedef/function names.)

Seems fine to me, but do you have a use for this?  If the answer is
yes then I'd be more confident that this is the right API.

> Another option would be to just change the old map function signature
> and break some existing client code.

possible but there's no absolute need to do this.

>>  Another bug that affects the iterator only is that the callback signature
>> is
>>
>>   typedef void* (*librdf_iterator_map_handler)(librdf_iterator *iterator,
>> void *map_context, void *item);
>>
>>  but it called with second two arguments swapped (rdf_iterator.c):
>>
>>       /* apply the map to the element  */
>>       element=map->fn(iterator, element, map->context);
>>
>>  I'm guessing this is wrong since the argument order is consistent in three
>> out of four cases: stream callback signature, stream callback call and
>> iterator callback signature.
> 
> Thanks. I fixed that one in svn r13842.
> 
> I guess the iterator map functions have not been used that much. This
> bug has been there since 2001.

Probably true.  However, the stream map functions are used to implement
various model methods such as get_targets, get_arcs when the underlying
storage only implements find_statements - the map iterator turns a
stream of statements into an iterator of nodes.

Dave