Simplifying the API

[tallesl]

Instead of:

public class MyRegistry : Registry
{
    public MyRegistry()
    {
        Schedule<MyJob>().ToRunOnceIn(5).Seconds();
        Schedule<MyOtherJob>().ToRunNow().AndEvery(2).Days();
    }
} 

protected void Application_Start()
{
    JobManager.Initialize(new MyRegistry()); 
}

We could just:

protected void Application_Start()
{
    JobManager.Schedule<MyJob>().ToRunOnceIn(5).Seconds();
    JobManager.Schedule<MyOtherJob>().ToRunNow().AndEvery(2).Days();
    JobManager.Start(); 
}

The latter is way simpler to use (not to mention that we could also ditch the AddJob method for late jobs, just use the Schedule whenever need).

One could argue that this way you can't have multiple registries implementation, but I bet that no one does that.

It's a little tricky to implement since the current JobManager is designed to work with an Registry (but it's worth the effort nonetheless).

[tallesl]

Also, instead of:

public class MyJobFactory : IJobFactory
{
    public IJob GetJobInstance<T>() where T : IJob
    {
        return MyDepencyInjectionFramework.GetInstance<T>();
    }
}

JobManager.JobFactory = new MyJobFactory();

We could just:

// "type" here is System.Type
JobManager.GetJobInstance = type => MyDepencyInjectionFramework.GetInstance(type) as IJob;

We lose the generics ("<T>") but a decent dependency injection framework (if not all of them) supports dealing with Type.

There would be no worry on the dependency injection framework returning null or the as conversion in the example doing so, the library would check for it and would throw a JobCreationException with a lovely message explaining what happened (instead of letting a NullReferenceException happen somewhere).

[Refresh06]

How come schedule is a generic that instantiates the job classes?
In my mind:

JobManager.Schedule(new MyJob(_someConstructorArg)).ToRun.......

Would be far more simple. JobFactory would be redundant, state could be hold from run to run (if needed) and injection is a breeze.

[tallesl]

Simpler indeed. But I don't get it how "injection is a breeze", I'm not seeing any dependency injection at all.

I don't really like the DI aspect of the library to be honest but I know people use it.

[Refresh06]

Regarding Injection, it's a lot simpler since the user of the API instantiate the job class themselves, they can use any injection framework they like without creating a job factory. I guess it's the same as saying the JobFactory is redundant ;o)

[tallesl]

Without "proper" DI in the library, you may have cases where the user have to call new MyJob(arg, anotherArg, yetAnotherArg, ...) and this may happen for several jobs. This can be even worse if some argument require DI itself to be constructed. Not to mention that user may not be happy with all the jobs being instantiated as soon it's scheduled (rather than at the time of the run).

I don't personally like that, I'm more on your side. Ditching DI completely is something tempting to do (for me), but I have to remember the folks that are using it. I'll consider doing it or not when closing this issue.

[Refresh06]

I actually disagree. A library should not be DI aware, it should just support DI by exposing the needed interfaces. A good rule of thumb is to never do new/activator.createinstance on a generic type.

What users of the library would have to do to use DI is something like kernel.getinstance<MyJob>(), which makes perfect sense if you are into DI.

As for instantiation, I would think that most people would like it better if the class was created once in stead of at every run, and if you really need a new instance at every run you could just new an inner calls up in the Execute method, but I think this would be a special case.

This also gives the option to call Dispose on .Stop if the Job class implements IDisposable.

[tallesl]

A library should not be DI aware,

Amen to that. That's my argument since, well, forever. A library design shouldn't entangle their design with yours. If it starts doing so you're in the way of making a framework (which is usually not a good thing in my book).

As I said, I never really liked the whole DI aspect of it. Heck, I never really used it except for testing the feature itself. The thing is, I'm not the original author of the library (#59), I didn't implement it.

But that's the problem no. 2 (and a piece of cake to solve it, despite being a major change). The no. 1 is by far the global scheduler, that I'm addressing on #148. While DI is more of a source of confusion than anything, the global scheduler is the reason of 99% bugs/bad pull requests I get.

A good rule of thumb is to never do new/activator.createinstance on a generic type.

Getting rid of the global scheduler makes the job factory (along with the Activator.CreateInstance call) goes away, which leaves no much benefit to the DI "slot" that we're bashing here.

Waiting until #148 to ditch DI brings both the benefit of a (yet another) justification to do so and of bundling together two major breaking changes.

P.S.: Giving a second thought, this issue is from the time that I wanted to simplify the API without changing how it fundamentally works. It was more like a note to myself. I'm closing it, but feel free keep the conversation going.

[Refresh06]

Sure thing, #148 makes perfect sense as well.
Thank you for a constructive conversation regarding the library, sounds like we are very aligned.
If you find the time to redesign the library and need some input, feel free to ask me anything.
I would love to contribute to the coding myself, but time is sparse atm...

[tallesl]

When it's done, being an early adopter and reporting issues would be an amazing contribution. Coding it's just one part of the equation. If you want to know more about the redesign stay tuned on #148.

Thanks for the conversation by the way!

[lloydjatkinson]

Are these links worth looking at? They are related to dependency injection "inside" a library.

http://stackoverflow.com/questions/2045904/dependency-inject-di-friendly-library
http://blog.ploeh.dk/2014/05/19/di-friendly-library/

[tallesl]

While the links are great to someone that is starting to grasp IoC (and DI for that matter), they don’t say much about why or when should a library be DI aware.

(I love how this guy had a breakdown)

In my experience, DI is a powerful tool when it comes to application architecture, but it's only useful when you have a well-defined core domain. The domain defines the interfaces, then other layers implement it (sometimes it’s useful to have more than one implementation of an interface). Then, just before the domain starts to run, you inject the implementation into the domain.

The domain defines only the interface, yet it still directly calls the injected implementation (even though it doesn’t know its type or from where it comes from). There you have it, the Inversion of Control.

But a library has nothing to do with that. Unless you are aiming for making more of a framework, a library shouldn’t dictate in any way the high level details of your architecture. A library is supposed to be used inside an implementation of yours of an interface of yours. You, the system designer and library user, should be 100% responsible of your DI design and usage.

Just imagine the mess if every library attempted to have some sort of DI awareness corner.

[Refresh06]

@tallesl I could not have said it better...

[nrjohnstone]

The issue here is not really whether a library should force you to use DI or not, but rather the fact that FluentScheduler resolves instances and executes them, it is in fact another composition root.

All obvious differences aside, FluentScheduler is no different to a web request being executed, and as such it should allow some manner of providing dependency injection so that each request (scheduled task) can be executed by the instance being resolved from a container.

This approach allows things like lifecycle scopes to be used effectively (ie transient versus scoped to current request versus singleton).

I currently use an implementation of IJobFactory that has a reference to my container (SimpleInjector) which does an ok job (pun intended) of allowing this but unfortunately has the design flaw of wanting to return a resolved instance outside of an async scope, so a bit more work is required if you are using request scope lifestyles.

Perhaps it might be an option to make FluentScheduler behind the scenes operate more like a webrequest where it will resolve jobs using the IJobsFactory but will allow hooks for creating "child containers" when per request life cycles are required.

As a final comment, please don't remove the current support for DI in this "library" because any library that can act as a composition root should provide (but not force/require) a way of using DI, and also I would consider FS a small framework for scheduling jobs, not a library.

[marcwittke]

like @nrjohnstone I was afraid of having the JobFactory removed, but I can confirm that after implementing this approach the design get's much clearer.

I am using SimpleInjector, too, and this is how it is working right now.

A IJobExecutor interface:

public interface IJobExecutor
{
    void ExecuteJob();
}

Some implementations of IJobExecutor acting as Simple Injector decorators for cross cutting concerns like unit of work handling, logging, exception handling, e.g.:

public class UnitOfWorkJobExecutor<TJob> : IJobExecutor<TJob>
{
    private static readonly ILogger Logger = LogManager.Create<UnitOfWorkJobExecutor<TJob>>();
    private readonly IUnitOfWork unitOfWork;
    private readonly IJobExecutor<TJob> jobExecutor;
    
    public UnitOfWorkJobExecutor(IUnitOfWork unitOfWork, IJobExecutor<TJob> jobExecutor)
    {
        Logger.Info($"Beginning unit of work for {typeof(TJob).Name}");
        this.unitOfWork = unitOfWork;
        this.jobExecutor = jobExecutor;
    }

    public void ExecuteJob()
    {
        try
        {
            unitOfWork.Begin();
            jobExecutor.ExecuteJob();
            Logger.Info($"Completing unit of work for {typeof(TJob).Name}");
            unitOfWork.Complete();
        } 
        catch
        {
            Logger.Info($"Aborting unit of work for {typeof(TJob).Name}");
            throw;
        }
        finally
        {
            unitOfWork.Dispose();
        }
    }
}

A small method resides in my application root class (ScopeManager and CompositionRoot are thin interface layers over SimpleInjector's Container)

private void ExecuteJob<TJob>() where TJob : IMyPersonalJob
{
    using (ScopeManager.BeginScope())
    {
        CompositionRoot.GetInstance<IJobExecutor<TJob>>().ExecuteJob();
    }
}

This method is used to add the jobs to the schedule:

JobManager.AddJob(ExecuteJob<SendSmtpJob>, s => s.ToRunNow().AndEvery((int) jobScheduleOptions.SendSmtpJobInterval.TotalSeconds).Seconds());

Today, JobExecutoris the nasty static class. Tomorrow it will be an instance hold by my application root class, which is an easy and natural step. Also vanished: An awkward SimpleInjectorJobFactory including MakeGenericType and Activator.CreateInstance - so I am happy with that approach.

[nrjohnstone]

@marcwittke perfect, that looks EXACTLY like what I want, just haven't had the time (or will) to sit down and fight with it... Thanks mate !

[nrjohnstone]

@tallesl you should take the time to make sure the documentation includes @marcwittke example above for designing your jobs as a composition root with an IoC container

[dotnetjunkie]

@tallesl,

I do agree with your sentiment about libraries having to be oblivious to DI. Mark Seemann wrote a great article about this, which, considering the above comments, you are already aware of.

FluentScheduler, however, is not a library. It is a framework. It is a framework in the same right that ASP.NET Core MVC and Nancy are (web) frameworks, and in the same right that NServiceBus and Rebus are (messaging) frameworks. The FluentScheduler framework is in control over the execution of our application code. This is Inversion of Control and that's the primary difference between a library and a framework.

With that in mind, frameworks will often require to be "DI aware". This doesn't mean a framework should dictate that a DI Container should be used, or even what the shape of a DI Container's API should be. That is a fallacy and an often made mistake that leads to the Conforming Container anti-pattern.

Instead, a framework should define interception points that allows users to plug in a DI Container of their choice, or use Pure DI if they rather use DI without the use of a container, or not use DI at all, if their application is truly simple. Mark Seemann, again, has some great guidance on making your framework DI friendly.

TBH, I think the current IJobFactory interface is quite close to what makes your framework DI friendly, although I think that an IJobExecutor abstraction, as suggested by @marcwittke makes integrating even simpler, as integrating scoping is harder when the framework invokes the job directly (as can be seen in my recent Stack Overflow answer). You could even add a few extension methods on top of that to simplify integration for the most common cases. That could potentially reduce integration to a one-liner in the majority of cases.

[tallesl]

@dotnetjunkie

Let's once again beat the dead horse.

FluentScheduler, however, is not a library. It is a framework.

Repeat with me: "FluentScheduler is not a framework."

The FluentScheduler framework is in control over the execution of our application code.

Once more: "FluentScheduler is not a framework."

It does control some of your code. But, usually, it's a tiny portion of the application code.

Unless you're redoing cron, the Windows Task Scheduler or something alike, users typically just sprinkle some schedules here or there in their (big) systems, in which most of the code is unrelated to scheduling.

This is Inversion of Control and that's the primary difference between a library and a framework.

One last time: "FluentScheduler is not a framework."

That's terrible definition. Take JavaScript's setTimeout:

setTimeout(function () { alert('a second just passed') }, 1000)

Is setTimeout a framework then? Defining not so black and white concepts in a short statement doesn't seem to suffice.

You know what else is (supposed to be) just a timer? This very library. FluentScheduler should be nothing more than a glorified easy-to-use no-frills timer.

Factories, managers, registries, interfaces. Needlessly abstractions muddling the water of the real value of the library. Thank god they'll all soon be gone (#214).

I purposely try to make my designs and discussions free from all that mindset of Java flavored OO illustrated by Martin Fowler blog posts. The cliché "simplicity is prerequisite for reliability" quote goes here.

[dotnetjunkie]

@tallesl,

I'm sorry for beating the dead horse. As I see it, FluentScheduler currently is a framework. But this is actually caused by it having an IJob interface that a user needs to implement (much like ASP.NET MVC requires your controllers to implement Controller). Because of the existence of this interface, the framework needs to define a seam that allows intercepting the creation of jobs, such as an IJobFactory. In other words, FleuntScheduler currently expects user code to conform to a contract it defines, which allows it to apply Inversion of Control, which makes it a framework.

If I understand correctly, your goal is to simplify FluentScheduler's API in such way that it effectively becomes a library again. This means removing the IJob and IJobFactory interfaces and letting users register their jobs with simple delegates.

I wholeheartedly agree with that approach, because libraries are often simpler compared to frameworks and cause less coupling between with application code. As I browsed through the code base, I noticed there's a lot of complexity in FluentScheduler that can be removed when you remove this interception model. Less complexity is a win for the maintainer and a win for the user. So double thumbs up for that.

The only thing that seems to be missing right now is good documentation examples that explain to users how to use integrate their DI Container effectively while registering delegates (or at least, I couldn't find any in the documentation). I did my share by updating my recent Stack Overflow answer to try to explain how to write an extension method for Simple Injector that simplifies integrating with FluentScheduler, with the aim of preventing code duplication. This might be of help to you and an inspiration to others using different DI libraries.

I wish you the best of luck.

[tallesl]

The only thing that seems to be missing right now is good documentation examples that explain to users how to use integrate their DI Container effectively while registering delegates (or at least, I couldn't find any in the documentation).

The only excuse to let this documentation burden on my shoulders (as the library maintainer) is due this nasty little line, that thankfully it's on its way out. Yet I find baffling how folks can't get around that on their own.

So you can't have a constructor with parameters (automatically resolved by your DI framework). How hard is to conclude that you're going to either access something static (that then resolves the instance) or get via closure (which is infinitely better)?

I believe the fault (mostly) lies with the complexity goo created by DI frameworks. Those fancy-pants concepts of "transient", "scoped" and "lifetime managers" makes programmers unable to think for themselves.

I sincerely don't know why we're still here on #71, when the real discussion took place on #148 which resulted in #214.

Rest in peace, 2016 issue.

[lloydjatkinson]

I'm not really involved in any of these issues but as I commented in the past I've been getting emails.

I think saying things like makes programmers unable to think for themselves. is just hurtful and totally unneeded. You obviously don't like DI but I don't think you need to start insulting people who use methodologies you don't "approve of".

[tallesl]

I'm locking this issue since its value is long gone plus people taking things personally.

I don't shy away from stating what I've experienced around here, even if it isn't smiles and sunshines. And remember, you're not your code.