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

Task Scheduler for small interval and one number of start

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

Problem

Scenario:

I want a task scheduler that enables my application to schedule some tasks in small time interval. I want to use it in my socket server application. For example, when the user connects to the socket application schedule some task in 15 seconds to disconnect user if no actions were made.

  • Queuing task for specific time



  • Ability to stop scheduled jobs



  • Running jobs in separate thread



So, I've decided to create my own Task Schedule. It can schedule tasks. It use System.Timers.Timer to find tasks to run. I want you to review my decision, may be solution has architecture errors etc.

public abstract class ScheduleJob
{
//Date when task must be executed
public DateTime DateTrigger { get; set; }

//Task Id (for search and cancel)
public string TaskId { get; set; }

//For logs
public abstract string TaskName { get; }

//Main task method
protected abstract void OnStart();

//Default task class delay (for example 15 seconds)
public abstract TimeSpan ScheduleTo { get; }

private bool _isRunning = false;

public void Start()
{
_isRunning = true;

var s = new Stopwatch();
s.Start();

try
{
//run task in separated thread
Task.Factory.StartNew(OnStart).ContinueWith(t =>
{
s.Stop();
_isRunning = false;

//if (t.Exception != null)
//{
//TODO work with exceptions
//}
});
}
catch (Exception ex)
{
//TODO work with exceptions
}
}
}


Manager to work with jobs

`public sealed class ScheduleManager : IDisposable
{
private static ScheduleManager _instance = new ScheduleManager();

public static ScheduleManager Instance { get { return _instance; } }

private List _jobs;
private object _lock = new object();

private Timer _timer;

private ScheduleManager()
{

Solution

Good work. A few comments and questions.

  • In SchduleJob class, I think that DateTrigger and TaskId need to be defined as internal and the SchduleManager only need to set them. If they will be public, the concrete class may set them to some value and the manager will override it in the Schedule method.



  • I think you need to constraint the TaskId to be unique and the manager only set it with a unique validation. For user define name\id you the TaskName property.



  • According to section 2, in the Remove method, replace RemoveAll by Remove.



  • Consider to change the type to long\int instead string, I don't talk about performance but about meaning although string is also make sense.



  • If you prefer to use string for the id, use string.CompareOrdinal() for the comprasion.



  • I think all the logging (add, remove, start, stop, error) need to be in the manager. The ScheduleJob need to be dedicated to the job. It can handle exception if you want, but the manager need to log it also.



  • Make the ScheduleManager IDisposable is dangerhere. Its a singleton and no body going to use it with using statement, but if somebody will do it, it make a problem because your timer will be disposable...

Context

StackExchange Code Review Q#114867, answer score: 2

Revisions (0)

No revisions yet.