Custom Task Scheduler

Question

I want a task scheduler with some specific capabilities for my application. I've searched for a good library that provides me with the following, but found nothing.

• Queuing awaitable tasks.
• Ability to stop started jobs.
• Ability to repeat some jobs.
• Ability to cancel repeat enabled jobs.
• Specifying a priority foreach schedule.
• Running schedulers on a separate thread [optionable].
• Add Task<string> and Task<bool>

So, I've decided to create my own and I want you to review my code for improvements:

• Detailed review of my code for errors, drawbacks.
• How I can make my code run every scheduler on a separate thread in an efficient way.
• Any more suggestions to help making my code more reliable and more professional, along with new features.

public static class TaskManager
{
    private static List<Scheduler> _schedulers;

    /// <summary>
    /// Gets a list of currently running schedulers.
    /// </summary>
    public static IEnumerable<Scheduler> RunningSchedulers
    {
        get { return _schedulers.Where(scheduler => scheduler.IsRunning()); }
    }

    /// <summary>
    /// Gets all scheduleres.
    /// </summary>
    public static IEnumerable<Scheduler> AllSchedulers(string name)
    {
        return _schedulers;
    }

    /// <summary>
    /// Gets a scheduler by it's name.
    /// </summary>
    public static Scheduler GetScheduler(string name)
    {
        return _schedulers.Find(scheduler => scheduler.Name == name);
    }

    /// <summary>
    /// Attempts to remove a scheduler from the list.
    /// </summary>
    public static bool RemoveScheduler(Scheduler scheduler)
    {
        return _schedulers.Remove(scheduler);
    }

    /// <summary>
    /// Creates a new scheduler and adds it the the list.
    /// </summary>
    public static Scheduler CreateScheduler(int priority, string name = null)
    {
        if (_schedulers == null)
            _schedulers = new List<Scheduler>();

        var scheduler = new Scheduler(priority, name);
        _schedulers.Add(scheduler);
        return scheduler;
    }
}

public class Scheduler : Queue<Job>
{
    public string Name { get; private set; }
    public int Priority { get; set; }
    private Timer _triggerTimer;
    public int Interval { get; private set; }
    public EventHandler<Job> Trigger;

    public Scheduler(int priority, string name = null)
    {
        Priority = priority;
        Name = name;
        Interval = 20;
    }

    private void TriggerTimerCallBack(object state)
    {
        foreach (Job job in this.Where(job => DateTime.Now >= job.Execution && !job.Triggered))
        {
            job.Triggered = true;
            if (job.Repeating)
                job.StartRepeating();
            Trigger(this, job);
        }
    }

    /// <summary>
    /// Sets the trigger timer interval.
    /// </summary>
    public void SetRefreshInterval(int interval)
    {
        Interval = interval;
    }

    /// <summary>
    /// Gets all the triggered jobs.
    /// </summary>
    public IEnumerable<Job> GetTriggeredJobs()
    {
        return this.Where(job => job.Triggered);
    }

    /// <summary>
    /// Gets all the repeat enabled jobs.
    /// </summary>
    public IEnumerable<Job> GetRepeatingJobs()
    {
        return this.Where(job => job.Repeating);
    }

    /// <summary>
    /// Returns whether the schedular is running.
    /// </summary>
    public bool IsRunning()
    {
        return _triggerTimer != null;
    }

    /// <summary>
    /// Stops the scheduler.
    /// </summary>
    public void Stop()
    {
        if (null == _triggerTimer) return;
        _triggerTimer.Dispose();
        _triggerTimer = null;
    }

    /// <summary>
    /// Starts the scheduler.
    /// </summary>
    public void Start()
    {
        if (_triggerTimer != null) return;
        _triggerTimer = new Timer(TriggerTimerCallBack, null, 0, Interval);
    }
}

public class Job : EventArgs, IDisposable
{
    public string Name { get; set; }
    public Func<Task> Task { get; set; }
    public DateTime Execution { get; private set; }
    public bool Triggered { get; set; }
    public bool Repeating { get; set; }
    private int _interval;
    private Timer _triggerTimer;

    public Job(Func<Task> task, DateTime executionTime, string name = null)
    {
        Name = name;
        Task = task;
        Execution = executionTime;
    }

    public Job TriggerEvery(int interval)
    {
        _interval = interval;
        Repeating = true;
        return this;
    }

    public Job Seconds()
    {
        _interval = (_interval)*1000;
        return this;
    }

    public Job Minutes()
    {
        _interval = _interval*(1000*60);
        return this;
    }

    public Job Hours()
    {
        _interval = _interval*(60*60*1000);
        return this;
    }

    public Job Days()
    {
        _interval = _interval*(60*60*24*1000);
        return this;
    }

    public void TriggerTimerCallBack(object sender)
    {
        if (Triggered)
            Triggered = false;
    }

    public void StartRepeating()
    {
        if (null != _triggerTimer) return;
        _triggerTimer = new Timer(TriggerTimerCallBack, null, _interval, _interval);
    }

    public void StopRepeating()
    {
        if (null == _triggerTimer) return;
        _triggerTimer.Dispose();
        _triggerTimer = null;
    }

    public void Dispose()
    {
        StopRepeating();
    }
}

Usage

#region Initializing Schedulers

var meetings = TaskManager.CreateScheduler(0, "Meeting Scheduler");
var jobs = TaskManager.CreateScheduler(1, "Work Scheduler");

#endregion

#region EventHandler Registration

meetings.Trigger += (sender, job) => Console.WriteLine(job.Name);
jobs.Trigger += (sender, job) => Console.WriteLine(job.Name);

#endregion


#region Adding Jobs

//One time meeting to be triggered after 15 seconds.
meetings.Add(new Job(Job, DateTime.Now.AddSeconds(15), "Meet Barack Obama"));

//Repeated meeting to be triggered after 5 days and repeated every 5 days.
meetings.Add(new Job(Job, DateTime.Now.AddDays(5), "Visit Your Dad").TriggerEvery(5).Days());

//Repeated job to be triggered after 5 minutes and repeated every 2 hours.
jobs.Add(new Job(Job, DateTime.Now.AddMinutes(5), "Make a cheese sandwich").TriggerEvery(2).Hours());

#endregion

#region Starting Schedulers

meetings.Start();
jobs.Start();

#endregion

#region Extra Features

//Get all running schedulers
var allRunningSchedulers = TaskManager.RunningSchedulers;

//Get all schedulers
var allSchedulers = TaskManager.AllSchedulers;

//Get specific scheduler by name
var meetingScheduler = TaskManager.GetScheduler("Meeting Scheduler");

//Remove specific scheduler
bool succeed = TaskManager.RemoveScheduler(meetingScheduler);

//Get repeating jobs from scheduler
IEnumerable<Job> steveJobs = meetingScheduler.GetRepeatingJobs();

//Get triggered jobs from scheduler
IEnumerable<Job> triggeredJobs = meetingScheduler.GetTriggeredJobs();

#endregion

Console.ReadLine();

Answer 1

The issues i see off the bat:

1) In my experience this Scheduler : Queue<Job> is almost always a bad idea. If you are not directly overriding a Queue class methods - there is no reason to derive Scheduler from it. Futhermore, adding/removing schedules will cause exceptions, if it happens at the same time, as you enumerate your queue in timer callback. You should use aggregation instead and synchronize your queue.

2) You should extract IJob interface. And use it in Scheduler. Because, for example, some guy might want to use your scheduler but he might not want to use Task class at all. Why force him? You could declare IJob like this (for example):

interface IJob : IDisposable
{
    string Name {get;}
    DateTime ExecutionTime {get;}

    void Start();
    void Cancel();

    bool Repeat {get;}
    bool InProgress {get;}

    event Action<IJob> Started;
    event Action<IJob> Completed;
    event Action<IJob, int> ProgressChanged;
}

It will give your scheduler and task manager all the control they need, without mentioning tasks at all. While you can still use tasks in your base implementation (Job class).

3) Same goes for IScheduler.

4) Methods such as public Job TriggerEvery(int interval), public Job Seconds(), etc. do not belong to Job class and will only lead to confusion. There are better ways. You can declare them as extension methods. You can implement job factory and impelent those expressions there. Either way you should avoid forcing a guy, who is going to use your code, to use those expressions. You should allow him to simply set _interval (via property or constructor).

5) private int _interval; what is int? Is it seconds? Is it ticks? Is it milliseconds? No way to tell without digging into your code. Use better naming, or use TimeSpan.

6) Job.StartRepeating(), Job.StopRepeating() - those do not belong to job class either. Job class should have an indicator whether the task should be repeated forever or not and thats it. It should not run itself or repeat itself - that is the job for scheduler/task manager. Not to mention that im not even sure, what those methods do and if they work.

7) You should never call events like that Trigger(this, job);. You can check Jeffrey Richter's books if you need details.

8) public bool IsRunning() this should be a property.

9) The idea behind your TaskManager is not quite clear. At the moment it looks like an overcomplicated dictionary.

10) Your Jobs are not being executed at any point. This is counter intuitive. If i add my job to scheduler/task manager, i expect it to run at designed time. Instead of triggering an event and let some external code handle the task itself, you should run the task with your scheduler/task manager instead (using the interface i suggested in (2) or your own). Because thats the whole point, no? You can still generate an event tho.