Search code examples
capi-designsoftware-designopaque-pointers

Iterator providing a "view" of its current element


The design issue I'm currently solving is to iterate over some region of memory and on each such iteration retrieve from that memory some meta-data the client is interested in. I see 2 solutions currently:

I.

struct queue;

struct queue_element_view{
    int id;
    char *description;
};

//0 - if ok, -1 - if end of queue reached
int next(struct queue*);

//0 - if ok, -1 - if end of queue reached    
int current_element_view(struct queue*, struct queue_element_view *);

So the queue opaque struct can be traversed via next function and since the queue elements are platform dependent and I want to keep the library cross platform I provided a platform-independent struct queue_element_view that is sensible on all platforms.

Drawback:

If client writes code like this:

struct queue *queue_ptr = //
struct queue_element_view current_out;
current_element_view(queue_ptr, &current_out);
//current_out now contains current's element meta data
next(queue_ptr);
//current_out now may contain unspecified data
//since queue's current element changed.

So calling to next after calling to current_element_view clobbers the current_out.

II.

struct queue;

struct queue_element_view;

struct queue_element_view *allocate_view(void); 

int * get_id(struct queue_element_view *);

char * get_description(struct queue_element_view *);

//0 - if ok, -1 - if end of queue reached
int next(struct queue*);

//0 - if ok, -1 - if end of queue reached    
int current_element_view(struct queue*, struct queue_element_view *);

In this case we have allocated struct queue_element_view and current_element_view copies data to the object pointed to by struct queue_element_view * so next does not clobber the data.

Drawbacks:

  1. It involves a function additional call to simply retrieve int and char * fields

  2. It makes testing public api more complicated.

So I'm kind of confused which one would be preferrable/readable? Probably there is another alternative?


Solution

  • Alternative I

    The perceived problem with alternative (I) is apparently that calling next() will cause data previously copied into a struct queue_element_view to become invalid, as a result of next() (possibly) deallocating memory.

    Solution: make sure next() does not do that. This may mean that you have to make a copy of the description string to put in the view, instead of just giving the client a copy of the original pointer itself. In this case it may be helpful to provide a function for releasing any internal allocations reflected in a struct queue_element_view, maybe something like this:

    void queue_element_view_clean(struct queue_element_view *view) {
        free(view->description);
    }
    

    This relieves the client from having to know details of what needs to be cleaned up, and how, and what doesn't. They can then retain the data as long as they want, cleaning it up when they decide they are done with it. That a call to next() will mean that they are no longer the data for the current element of the iteration is a feature, not a bug -- why would it make sense to interfere with clients retaining data from previous iterations if they want to do so?

    Alternative II

    The perceived problems revolve around accesses to the members of the view going through functions. It's unclear how this solves the perceived problem with alternative I. Although it could be part of a solution for that problem, I see no reason to think it a necessary part.

    Solution: use alternative I. Seriously. If you're going to make copies of the data as needed for the view to persist appropriately when next() is called, then I don't see how you gain anything by making the view structure opaque.

    Overall

    Your two alternatives seem oddly flipped.

    It would make sense to me to use accessor functions if you wanted to avoid having a separate view structure or copying data. The functions would return data pertaining to the current element of the iteration - no separate view structure involved. You would then have the alternative of making the accessors provide copies of the data for which the caller takes responsibility, or making it a caller responsibility to copy any data they want to retain when the iterator is stepped forward.

    On the other hand, if you provide a separate structure for an element view then it seems odd that you would do so in a way that allows it to become invalid when the iterator is advanced. The separate view object seems a natural way to go for allowing the view data to be preserved as long as the caller wants it.

    Any way around, yes, some kind of responsibility is placed on the caller. This is natural -- there is no free lunch. Document clearly what those responsibilities are, and try to design the overall API in a way that feels consistent with respect to what kinds of responsibility are placed on the user, under what circumstances, and how those responsibilities are to be discharged.