Remove JobScheduler singleton
LilithSilver opened this issue · 2 comments
Is there a reason other than convenience that JobScheduler produces a singleton?
Instance
isn't referenced anywhere in the code except for IJobExtensions.Schedule()
, which is just a convenience method to avoid scheduler.Schedule(job)
. It doesn't even allow the client to avoid keeping a reference to the scheduler or referencing the instance themself, because they still need to call scheduler.Flush()
and scheduler.Dispose()
neither of which which have an extension method (and no extension method would make sense).
It seems to me it adds complexity, testing issues, and lack of thread safety.
Imagine the following...
var scheduler1 = new JobScheduler(...);
// somewhere completely else, or on a different thread:
var scheduler2 = new JobScheduler(...);
scheduler2.Dispose();
myJob.Schedule(...); // schedules on an invalid, disposed JobScheduler, even if you might expect the first!
I would argue we remove the Instance
and instead let the user track their own singleton instead, if they wish. If that sounds OK, let me know, and I can make a PR!
That's a good question. Originally I thought that a singleton would make more sense because there are not more than N threads anyway. And that it might make it easier to use.
But in the end you're right, maybe we should do without it instead ^^ So yes, a PR would make sense here.
Will do. I'm working on a refactor of JobScheduler to add dependency support, so I'll bundle this change into there, once I get the code up and running.