From 00dfd0d07cbb23ad637a99ab72d528fc61767219 Mon Sep 17 00:00:00 2001 From: Austin Bales Date: Sat, 16 Jul 2011 12:00:02 -0700 Subject: [PATCH 1/2] In PeriodicalExcuter, removed unnecessary `currentlyExecuting` ivar and other code relating to parallel execution. --- src/prototype/lang/periodical_executer.js | 31 +++++++---------------- test/unit/periodical_executer_test.js | 20 --------------- 2 files changed, 9 insertions(+), 42 deletions(-) diff --git a/src/prototype/lang/periodical_executer.js b/src/prototype/lang/periodical_executer.js index aa3b97715..49bc02671 100644 --- a/src/prototype/lang/periodical_executer.js +++ b/src/prototype/lang/periodical_executer.js @@ -31,13 +31,18 @@ var PeriodicalExecuter = Class.create({ initialize: function(callback, frequency) { this.callback = callback; this.frequency = frequency; - this.currentlyExecuting = false; - this.registerCallback(); + this.start(); }, - registerCallback: function() { - this.timer = setInterval(this.onTimerEvent.bind(this), this.frequency * 1000); + /** + * PeriodicalExecuter#start() -> undefined + * + * Starts the [[PeriodicalExecuter]] or restarts it if `#stop` + * was previously called. + **/ + start: function() { + this.timer = setInterval(this.execute.bind(this), this.frequency * 1000); }, execute: function() { @@ -71,22 +76,4 @@ var PeriodicalExecuter = Class.create({ clearInterval(this.timer); this.timer = null; }, - - onTimerEvent: function() { - if (!this.currentlyExecuting) { - // IE doesn't support `finally` statements unless all errors are caught. - // We mimic the behaviour of `finally` statements by duplicating code - // that would belong in it. First at the bottom of the `try` statement - // (for errorless cases). Secondly, inside a `catch` statement which - // rethrows any caught errors. - try { - this.currentlyExecuting = true; - this.execute(); - this.currentlyExecuting = false; - } catch(e) { - this.currentlyExecuting = false; - throw e; - } - } - } }); diff --git a/test/unit/periodical_executer_test.js b/test/unit/periodical_executer_test.js index 4fa19099b..6ce6ba955 100644 --- a/test/unit/periodical_executer_test.js +++ b/test/unit/periodical_executer_test.js @@ -12,24 +12,4 @@ new Test.Unit.Runner({ this.assertEqual(3, peEventCount); }); }, - - testOnTimerEventMethod: function() { - var testcase = this, - pe = { - onTimerEvent: PeriodicalExecuter.prototype.onTimerEvent, - execute: function() { - testcase.assert(pe.currentlyExecuting); - } - }; - - pe.onTimerEvent(); - this.assert(!pe.currentlyExecuting); - - pe.execute = function() { - testcase.assert(pe.currentlyExecuting); - throw new Error() - } - this.assertRaise('Error', pe.onTimerEvent.bind(pe)); - this.assert(!pe.currentlyExecuting); - } }); \ No newline at end of file From 2b67f9e1be0658cc9eab9ac6416020a951362fed Mon Sep 17 00:00:00 2001 From: Austin Bales Date: Sat, 16 Jul 2011 12:10:22 -0700 Subject: [PATCH 2/2] Check to ensure that `this.timer` doesn't exist before assigning it in `#start` --- src/prototype/lang/periodical_executer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/prototype/lang/periodical_executer.js b/src/prototype/lang/periodical_executer.js index 49bc02671..5eb79220b 100644 --- a/src/prototype/lang/periodical_executer.js +++ b/src/prototype/lang/periodical_executer.js @@ -42,7 +42,7 @@ var PeriodicalExecuter = Class.create({ * was previously called. **/ start: function() { - this.timer = setInterval(this.execute.bind(this), this.frequency * 1000); + if (!this.timer) this.timer = setInterval(this.execute.bind(this), this.frequency * 1000); }, execute: function() {