Skip to content

Rename enable() and enableDelayed() #63

@GaryBoone

Description

@GaryBoone

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions