Closed GaryBoone closed 5 years ago
Hi,
Thank you for a thoughtful comment.
enable
is designed the way it is because in my personal practice more tasks needed immediate execution than a delayed one. Therefore the default behavior is such.
It is explicitly stated in the documentation and should be taken into consideration during coding: enable
is an explicit request to turn the task on with all the prep work required (including execution of the OneEnable
method) and request the scheduler to invoke it as soon as its position in the execution chain permits.
Disable
on the contrary is only executed once (for roughly the same reason).
I must point out that you are the first one to suggest that this may be confusing.
To answer your specific concerns:
There is a separate method: enableIfNot()
for situations where a programmer is not certain. This can save you an if statement.
As for the incorrect timing, the first execution is almost always at a mercy of when the task was enabled and when the first scheduling pass happens to occur.
In a cooperative multitasking environment, the exact timing of a task invitation could never be guaranteed anyway. If exact timing is a requirement you need to consider other pre-emptive solutions.
Again, no one else complained about the function name is misleading.
OnEnable
to run if a programmer decides task should be re-enabled. I use explicit enable
or restart
to explicitly reset the task schedule start point. Changing such fundamental behavior concept would make TaskScheduler incompatible with all the code already written. Now that would lead to confusion and bugs. Backward compatibility was always my primary concern, so I am afraid for that reason alone I am not going to change enable's behavior in such dramatic fashion.Again, there is enableIfNot
for exactly the cases you described, which I actually used a couple of times myself.
Having said all that, I will consider what of your suggestions I could incorporate into the new version of TaskScheduler. Thank you for your feedback. (And you are always free to fork and refactor TS to your liking - the beauty of open source).
Cheers!
My 2 cents on the issue.
And yet, according to Merriam-Webster
enable: to cause (a feature or capability of a computer) to be active or available for use
https://www.merriam-webster.com/dictionary/enable
Anyway, backward-compatibility kind of seals the deal. I have literally thousands of lines of TS-based code that assumes immediate execution. Afraid I can't change enable
so radically.
The TaskScheduler is well-designed in allowing both immediate execution of tasks via enable() or delayed execution via enableDelayed(). The documentation for enable() says that it "enables the task, and schedules it for immediate execution (without delay) at this or next scheduling pass depending on when the task was enabled." That's likely to cause confusion and bugs, however, for a few reasons:
1) running tasks immediately upon enable() means that the may execute at incorrect times. For example, a task may be set to run once per second, but if the enable() is called more than once in a second, the task will run more than once per second. This semantics means that the default behavior can lead to bugs. Programmers have additional burden to code this correctly.
2) "Enable" commonly means to make possible, not to do. So the function name is misleading.
3) For the same reason, enable is a function that should be idempotent. Intuitively, if you enable something that is already enabled, nothing should happen.
4) It doesn't scale. Suppose we have a thousand Tasks all with different timings. Suppose further that enabling them all via with enableDelayed() works: they all run correctly at their appointed times. But with the current semantics, using enable() and all of them would make them all run immediately, likely leading to conflicts or overload.
Therefore enable() should use the delay, and enableImmediate() should be created to run immediately. Or to minimize disruption for existing users, deprecate enable(), leaving enableImmediate() and enableDelayed() to maximize explicitness and clarity.