Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Unified Method to Add Jobs in Scheduler #228

Open
jakmaz opened this issue Apr 13, 2024 · 3 comments
Open

Feature Request: Unified Method to Add Jobs in Scheduler #228

jakmaz opened this issue Apr 13, 2024 · 3 comments
Assignees

Comments

@jakmaz
Copy link

jakmaz commented Apr 13, 2024

Description:
The current implementation of the Scheduler class requires checking the type of each job (either SimpleIntervalJob or CronJob) to determine which add method to call. This design necessitates additional boilerplate code and type handling on the client's side.

    if (job instanceof IntervalJob) {
        this.scheduler.addIntervalJob(job);
    } else if (job instanceof CronJob) {
        this.scheduler.addCronJob(job);
    } else {
        throw new Error("Unsupported job type");
    }

Suggested Improvement:
I propose adding a unified method, addJob, to the Scheduler class to handle the addition of jobs regardless of their type. This method would internally check the job type and delegate to the appropriate existing method (addSimpleIntervalJob or addCronJob).

Benefits:
This change would simplify the Scheduler's usage by abstracting away the need for explicit type checks in client code, making the API cleaner and easier to use.

Request:
I would like to be assigned to this issue to implement the proposed feature, as I have already identified the necessary changes and am prepared to contribute the enhancement.

@kibertoad
Copy link
Owner

@jakmaz I've been wanting to do this for a while. Please go for it!

@jakmaz
Copy link
Author

jakmaz commented Apr 13, 2024

Alright!
Do you want me to leave the previous methods such as: addIntervalJob(), addLongIntervalJob(), addSimpleIntervalJob(), addCronJob() public and still usable or make them private and rewrite the tests for new addJob()?

@kibertoad
Copy link
Owner

mark them as deprecated, we'll drop them in semver major

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants