John Darrington <[email protected]> writes: > On Sat, Jul 18, 2009 at 09:39:38PM -0700, Ben Pfaff wrote: > > I don't really understand the algorithm used by > casereader_create_distinct. Why are there two layers > (casereader_create_filter_func and casereader_create_translator)? > > Ease of implementation I think. > > Why is it necessary to use casereader_peek (which forces all of > the input data to stay around) instead of reading cases one at a > time (and possibly being able to throw away data that has been > consumed)? > > I couldn't find any other solution which actually worked.
OK. I think that this can be improved, but it doesn't have to be for now. > process_group does a *lot* of manipulation of cases and > caseprotos and readers and writers. I didn't check it in detail > because it wasn't obvious to me what the overall goal was. It > could really use a high-level comment. > > I added a comment which explains that it actually does 3 things. In > order to keep down the number of data passes, it's necessary to have this > multi-purpose function. Thanks for adding the comments. In "Respect the constness of caseproto." I don't understand the benefit of the new caseproto_clone function. As far as I can tell there is never any benefit in calling it, because it is always equivalent to simply calling caseproto_ref, except that with caseproto_clone the caller doesn't get the benefit of lazy reallocation or lazy refreshing of the long string cache. Maybe this property of caseproto_ref could be made clearer if caseproto_clone were simply implemented as "return caseproto_ref (proto);"? (Note that this property is true only because every function that modifies a caseproto returns a new caseproto that replaces it. Otherwise we'd need a public caseproto_unshare function, like case_unshare, and functions that modify caseprotos would only accept caseprotos with refcount 1.) I still don't see anything that unrefs cdr->proto when the casereader is destroyed. Does it happen somewhere that I'm not seeing? In "Fix cleanup of ROC command." all of the added goto statement have double-semicolons: "goto error;;". -- Ben Pfaff http://benpfaff.org _______________________________________________ pspp-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/pspp-dev
