2009/4/8 Mike Rylander <mrylan...@gmail.com>: > On Wed, Apr 8, 2009 at 12:21 AM, Dan Scott <deni...@gmail.com> wrote: <snip>
>> I've fixed these issues in r12816 using a minimalist approach, but >> it's not clear to me why the current implementation is as complex as >> it is. ingest.full.biblio.record_list seems to be a copy of >> ingest.full.biblio.record, with a bit of extra code to iterate over >> the incoming list and to return a count of the records that were >> successfully processed. > > They could certainly be coalesced into a single implementation. The > only important difference is null vs 0 on a single (or all) record > error. Calling code looks at the return value of both as true/false, > so returning the bib id from the non-list version isn't needed. That > being said, it would not be hard to retain both existing return types > based on $self->api_name. Ah, the $self->api_name trick; that would be a good direction to go. Thanks for the suggestion, Mike. >> Replacing it with an implementation that just >> iterates over ingest.full.biblio.record for each element of the list >> (below) works just as well, and (at least to my eyes) is easier to >> read. >> > > [snip implementation] > > For long lists of record ids, the extra round-tripping of a retrieval > per record can cause significant slowdown. There was once a version > that was much like what you have there, but I rewrote it to do batch > re-ingests more efficiently. Oh, that makes perfect sense (of course). If we touch that area of code again without refactoring it away entirely, we should probably add a comment to that effect so that when Evergreen developer archaelogists from the year 3000 dig into the code, they won't run through the same rediscovery process. :) -- Dan Scott Laurentian University