Hi Minh Thanks for the code suggestions yet again! Just made the changes and pushed it out - it's all smart stuff. And credit where credit's due - James Healy was the one who wrote this feature, all I had to do was merge the change in.
Cheers -- Pat On 16/10/2008, at 5:00 AM, Minh Tran wrote: > > Hi Pat, > > I'm glad suspended_delta() is available in the master branch now; I've > been monkey patching to get something like it for a while, so thank > you! > > I looked at the source code, and it was simple and clean, but may I > suggest some best practices? I'll list each separately and provide my > rationale. Thank you ahead of time for your patience. The original > source code is pasted below for your convenience. > > === > def suspended_delta(&block) > ThinkingSphinx.deltas_enabled = false > yield > ThinkingSphinx.deltas_enabled = true > self.index_delta > end > === > > First, I think we should wrap the yield in a begin/ensure block so > that if there is an error, deltas will be re-enabled before the error > gets passed up the stack. This is potentially important because the > error may be caught and handled intelligently (in a way that requires > a re-index). > > Second, I think we shouldn't assume that deltas were enabled already; > above, the suspended_delta() code *always* sets deltas_enabled to true > rather than returns it to its previous value. Eliminating this > assumption doesn't make the code much more complex but will make the > method more robust. > > If you find any of this agreeable, I included a patch at the end of > this post. > > As always, Pat, thank you for your work on this! Hope you are doing > well =). > > Sincerely, > Minh Tran > > ===== > > --- a/lib/thinking_sphinx/active_record/delta.rb > +++ b/lib/thinking_sphinx/active_record/delta.rb > @@ -28,10 +28,14 @@ module ThinkingSphinx > # end > # > def suspended_delta(&block) > + orig_deltas = ThinkingSphinx.deltas_enabled > ThinkingSphinx.deltas_enabled = false > - yield > - ThinkingSphinx.deltas_enabled = true > - self.index_delta > + begin > + yield > + ensure > + ThinkingSphinx.deltas_enabled = orig_deltas > + self.index_delta > + end > end > > # Build the delta index for the related model. This won't > be called > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Thinking Sphinx" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/thinking-sphinx?hl=en -~----------~----~----~----~------~----~------~--~---
