Reputation: 5269
Well in an application I am creating I wanted to have a scheduler that holds a data about incoming jobs and the time remaining for them to be executed so I have created a very naive implementation that I think is far from what it should be.
Interfaces
public interface IQueue
{
string Name { get; set; }
int Priority { get; set; }
}
Classes
public class TaskScheduler : Queue, IQueue
{
public string Name { get; set; }
public int Priority { get; set; }
public TaskScheduler(string name, int priority)
{
Name = name;
Priority = priority;
}
public void Add(string work, TimeSpan execution)
{
Enqueue(new Job {Work = work, Execution = execution});
}
public Job Get()
{
return (Job) Dequeue();
}
}
public class Job
{
public string Work { get; set; }
public TimeSpan Execution { get; set; }
public override string ToString()
{
return string.Format("{0} will be excuted in {1}", Work, Execution);
}
}
Usage
var schedulerss = new List<TaskScheduler>
{
new TaskScheduler("Meetings", 2),
new TaskScheduler("Jobs", 1)
};
schedulerss = schedulerss.OrderBy(element => element.Priority).ToList(); //ORDER THE schedulers according to the Priority
schedulerss.Find(schedulers => schedulers.Name == "Meetings").Add("Meet Barack Obama", new TimeSpan(1, 0, 0, 15));
schedulerss.Find(schedulers => schedulers.Name == "Jobs").Add("Make a cheese sandwich :D", new TimeSpan(0, 2, 0, 15));
var meetingschedulers = schedulerss.Find(schedulers => schedulers.Name == "Meetings");
if (null != meetingschedulers)
{
foreach (var job in meetingschedulers)
{
Console.WriteLine(job);
}
}
Console.Read();
Will this code work well or I just missed all the thing out and there is better approach(s) to do such thing?
Please I would like a very detailed answer about what is the drawback(s) of the code I have provided and how could I create a better reused code (just for other people who may find this topic useful).
Upvotes: 5
Views: 5423
Reputation: 8758
Well as you can see by your usage, having the name of the scheduler inside the scheduler makes the code a bit funky... Does the scheduler really needs to know his own name? I'd use a Dictionary<string, TaskScheduler>()
instead, and put their names there..
Also your queue always returns a Job
, so you should use a generic queue, Queue<Job>
I've made some modifications
Interfaces
public interface IQueue
{
int Priority { get; set; }
}
Classes
public class TaskScheduler : Queue<Job>, IQueue
{
public int Priority { get; set; }
public TaskScheduler(int priority)
{
Priority = priority;
}
public void Add(string work, TimeSpan execution)
{
Enqueue(new Job { Work = work, Execution = execution });
}
public Job Get()
{
return Dequeue();
}
}
public class Job
{
public string Work { get; set; }
public TimeSpan Execution { get; set; }
public override string ToString()
{
return string.Format("{0} will be excuted in {1}", Work, Execution);
}
}
Usage
var schedulers = new Dictionary<string, TaskScheduler>();
schedulers.Add("Meetings", new TaskScheduler(2));
schedulers.Add("Jobs", new TaskScheduler(1));
schedulers["Meetings"].Add("Meet Barack Obama", new TimeSpan(1, 0, 0, 15));
schedulers["Jobs"].Add("Make a cheese sandwich :D", new TimeSpan(0, 2, 0, 15));
if (schedulers.ContainsKey("Meetings"))
{
foreach (var job in schedulers["Meetings"])
{
Console.WriteLine(job);
}
}
Since you do work with multiple schedulers, I'd suggest to make some sort of schedulerController, which will handle prioritizing for you, and other things you might want to add.
I don't know exactly you want to do with your schedulers, but I meant something like this, a utility class:
public class TaskSchedulerController
{
private Dictionary<string, TaskScheduler> _scedulers;
public TaskSchedulerController()
{
_scedulers = new Dictionary<string, TaskScheduler>();
}
public void Add(string name, int priority)
{
_scedulers.Add(name, new TaskScheduler(priority));
}
public IEnumerable<string> GetJobsOfScheduler(string name)
{
if (_scedulers.ContainsKey(name))
{
foreach (var job in _scedulers[name])
{
yield return job.ToString();
}
}
}
}
Upvotes: 2
Reputation: 1
I see some issues with your abstraction... It appears that you abstract the queue, but not the Job class. This should be the opposite (or at least your life will be a lot easier that way).
public interface IJob
{
MyJobType Type{ get; set; }
string Name { get; set; }
int Priority { get; set; }
void RunJob();
}
You can remove IQueue since you are not using it anyway.
Use only one queue unless there is a good reason not solved by #3
Abstract your job class - after all the Job that creates the cheese sandwiches will not be the same as the one that handles meetings. With something like the interface above you can order jobs by type (an enumeration you create) and priority, then run them using IJob.RunJob without having to know what they actually do in each implementation.
Upvotes: 0