HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Flow Task Scheduler

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
schedulerflowtask

Problem

Scenario:

I wanted a task scheduler/doer that enables my application to schedule some tasks to be executed in a specified time but in the same order they were scheduled in or depending on their priorities (DataFlow like). I also wanted it to enable the usage of C# 5 Async/Await functionality so I looked up over the Internet and didn't found something the truly fit my needs so I decided to implement one... a simple yet advanced (in my opinion) task scheduler that exactly achieved what I want. However, I want to improve it by hearing your opinions about it.

Implementation:

1) Scheduler Class

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace ATScheduler
{
public abstract class Scheduler
{
#region Fields

private readonly TimeSpan _baseInterval;
private readonly TimeSpan _executionTimeOut;
private readonly List _jobs;
private readonly object _sync;
private readonly Timer _timer;
private bool _paused;

#endregion

#region Constructors

protected Scheduler()
{
_jobs = new List();
_sync = new object();
_baseInterval = TimeSpan.FromMilliseconds(12000);
_executionTimeOut = TimeSpan.FromMilliseconds(30000);
_timer = new Timer(ProcessJobs, null, Timeout.Infinite, Timeout.Infinite);
}

#endregion

#region Methods

public abstract Task JobTriggered(IJob job);
public abstract Task JobException(IJob job, Exception exception);

private void ProcessJobs(object state)
{
lock (_sync) {
IJob[] jobsToExecute = (from jobs in _jobs
where jobs.StartTime first.StartTime.CompareTo(second.StartTime));
TimeSpan delay = _jobs[0].StartTime.Subtract(DateTimeOffset.Now);
long dueTime = Math.Mi

Solution

Just a couple of notes.

public void PauseScheduler()
{
    if (!_paused) {
        _paused = !_paused;
    }
}

public void ResumeScheduler()
{
    if (_paused) {
        _paused = !_paused;
    }
}


Can be

public void PauseScheduler()
{
    _paused = true;
}

public void ResumeScheduler()
{
    _paused = false;
}


The test here is unnecessary

if (jobsToExecute.Any()) {
    foreach (IJob job in jobsToExecute) {


It would also be good to remove the magic numbers here:

_baseInterval = TimeSpan.FromMilliseconds(12000);
_executionTimeOut = TimeSpan.FromMilliseconds(30000);


(On that note, why not TimeSpan.FromSeconds(12) and TimeSpan.FromSeconds(30)?)

It seems that _jobs is always sorted by StartTime. If so, you could insert into the list at the appropriate place here instead of re-sorting the entire list

_jobs.Add(job);
_jobs.Sort((first, second) => first.StartTime.CompareTo(second.StartTime));


job would be a better name than jobs here:

from jobs in _jobs where ...


It's poor style to throw System.Exception. Create a custom exception class instead.

if (job.Repeat) {
    throw new Exception("Repeatable jobs can't be awaited!");
}

Code Snippets

public void PauseScheduler()
{
    if (!_paused) {
        _paused = !_paused;
    }
}

public void ResumeScheduler()
{
    if (_paused) {
        _paused = !_paused;
    }
}
public void PauseScheduler()
{
    _paused = true;
}

public void ResumeScheduler()
{
    _paused = false;
}
if (jobsToExecute.Any()) {
    foreach (IJob job in jobsToExecute) {
_baseInterval = TimeSpan.FromMilliseconds(12000);
_executionTimeOut = TimeSpan.FromMilliseconds(30000);
_jobs.Add(job);
_jobs.Sort((first, second) => first.StartTime.CompareTo(second.StartTime));

Context

StackExchange Code Review Q#59559, answer score: 3

Revisions (0)

No revisions yet.