Sorry, meant to say: I also didn't see anything wrong with using a variable to refer to `this` and not using #bind. If you do that, though, I'd recommend using the same variable name every time you do this, throughout all of your classes. The one I use is `self` because lots of people seem to use it for this (no pun). (Another one you see sometimes is `that`, but for me, that's just completely wrong [again, no pun].)
-- T.J. :-) On Apr 11, 7:20 am, "T.J. Crowder" <t...@crowdersoftware.com> wrote: > Hi, > > Off-the-cuff, I'd probably use two binds and one local (this is > totally off-the-cuff, and apologies if I've messed something up. Also, > we have slightly different coding styles and while I tried to stick to > what I inferred from what I saw, it's probably ended up a mish-mash): > > http://pastie.org/913777 (update1.js; also pasted at the end of this > message) > > I *might* grab the bound updateComplete function to a local variable > rather than re-binding it on every update, I think it may depend on my > mood. :-) (I didn't in the above, and of course as I write this I > think I should have.) > > For this, I probably wouldn't use a periodical executer, either, > because of all the state flags required. I'd probably go with chained > execution: > > http://pastie.org/913778 (update2.js; also pasted at end of message) > > ...but note that that change changes the timing; 'frequency' becomes > the time between the end of one update and the start of the next, > rather than the time between the start of updates (barring an update > taking longer than the time, in which case the earlier code skipped > that update entirely). > > *Totally* FWIW. > > Oh, and I'm a named function guy (I notice you're using an anonymous > function for your `autoUpdate` property); here's > why:http://blog.niftysnippets.org/2010/03/anonymouses-anonymous.html > > Happy coding, > > -- T.J. :-) > > **** update1.js: > autoUpdate: function(was_in_progress) { > > // TJC: One of the few situations where I find multiple > exits maintainable; early shortcutting > if (this.periodical_executer.updater) { > alert('There is a request currently in progress. > Please try again later or refresh the page.'); > return false; // TJC: Side note, the function > appears to return false (here) or undefined (main case) - subtle! ;-) > } > > var frequency = $F('page_updater_frequency'); > this.periodical_executer.updater = new > PeriodicalExecuter(doUpdate.bind(this), frequency); > > // TJC: To me, breaking this out in a named function > improves clarity > function doUpdate() { > if (!this.periodical_executer.request_in_progress) { > this.params = {'authenticity_token': > Utilities.getAuthToken(), > 'was_in_progress': > was_in_progress}; > this.periodical_executer.request_in_progress = > true; > new Ajax.Request(_location(), { > method: 'get', > parameters: this.getParams(), > onComplete: updateComplete.bind(this) > }); > } > } > > // TJC: Again, the named function (for me) immproves > clarity > function updateComplete(r) { > this.periodical_executer.request_in_progress = false; > var json = Utilities.updateElements(r); > if (json.js_data && json.js_data.should_stop) { > this.periodical_executer.updater.stop(); > var effected = this.periodical_executer.glowing; > if (effected) { > // stop glowing effect > effected.stopGlow(); > } > this.periodical_executer = {}; > } > else { > // typewriter effect for updated info > // TJC: Since it's just these IDs we need in the > anon function below, grab them > var tw_ids = this.type_writer_ids; > var for_typewriter = > json.partials.select(function(e) { return twids.include(e.id); }); > for_typewriter.each(function(e) { > _typeWriter(e.id); > }) > } > } > } > > *** update2.js: > autoUpdate: function(was_in_progress) { > > // Short-circuit if already running > if (this.periodical_executer.updating) { > alert('There is a request currently in progress. > Please try again later or refresh the page.'); > return; > } > > // Setup > this.periodical_executer.updating true; > var frequency = $F('page_updater_frequency'); > var updateComplete = updateCompleteImpl.bind(this); > var doUpdate = doUpdateImpl.bind(this); > > // Kick off the process > doUpdate(); // Or: doUpdate.delay(frequency); > > // Process an update > function doUpdateImpl() { > this.params = {'authenticity_token': > Utilities.getAuthToken(), > 'was_in_progress': was_in_progress}; > new Ajax.Request(_location(), { > method: 'get', > parameters: this.getParams(), > onComplete: updateComplete > }); > } > > // Process update completion > function updateCompleteImpl(r) { > var json = Utilities.updateElements(r); > if (json.js_data && json.js_data.should_stop) { > var effected = this.periodical_executer.glowing; > if (effected) { > // stop glowing effect > effected.stopGlow(); > } > this.periodical_executer = {}; > } > else { > // typewriter effect for updated info > var tw_ids = this.type_writer_ids; > var for_typewriter = > json.partials.select(function(e) { return twids.include(e.id); }); > for_typewriter.each(function(e) { > _typeWriter(e.id); > }) > > // Kick off next update > doUpdate.delay(frequency); > } > } > } > > On Apr 11, 5:28 am, patrick <patrick99...@gmail.com> wrote: > > > > > Hi, > > > This is one of my more 'complicated' methods that just seem like > > using .bind(this) at the end of the inner functions would make the > > code read more confusing.. I believe I have some other methods > > somewhere that actually pass functions as params inside those > > beforeCreate, afterFinish blocks, and they too would require .bind -- > > and at the point it just gets really ugly... > > > autoUpdate: function(was_in_progress) { > > var frequency = $F('page_updater_frequency'); > > var admin_table = this; > > > if (!this.periodical_executer.updater) { > > admin_table.periodical_executer.updater = > > new > > PeriodicalExecuter(function() { > > if > > (!admin_table.periodical_executer.request_in_progress) { > > admin_table.params = > > {'authenticity_token': > > Utilities.getAuthToken(), > > > > 'was_in_progress': was_in_progress}; > > > new > > Ajax.Request(_location(), { > > method: 'get', > > parameters: > > admin_table.getParams(), > > onCreate: > > function() { > > > > admin_table.periodical_executer.request_in_progress = true; > > }, > > onComplete: > > function(r) { > > > > admin_table.periodical_executer.request_in_progress = false; > > var json = > > Utilities.updateElements(r); > > if > > (json.js_data && json.js_data.should_stop) { > > > > admin_table.periodical_executer.updater.stop(); > > > var > > effected = admin_table.periodical_executer.glowing; > > if > > (effected) { > > > > // stop glowing effect > > > > effected.stopGlow(); > > } > > > > > admin_table.periodical_executer = {}; > > } > > else { > > // > > typewriter effect for updated info > > var > > for_typewriter = json.partials.select(function(e) > > { return... > > read more » -- You received this message because you are subscribed to the Google Groups "Prototype & script.aculo.us" group. To post to this group, send email to prototype-scriptacul...@googlegroups.com. To unsubscribe from this group, send email to prototype-scriptaculous+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/prototype-scriptaculous?hl=en.
